Fix the project-directory advice #2283 added

[schnie]

Follow-up to #2283, fixing what review found in the "not an Astro project" advice it added. The first commit fixes the first review round, the second and third a second round, the fourth a third round, and the fifth a final round. Where rounds disagree, the later round is what the code does now: the fifth replaced every home-directory special case with the one rule below, so read that first.

The rule (6ce97ec)

The home directory is an ordinary directory, and the CLI's settings file is never a project's.

• A directory is an Astro project when it has a pyproject.toml carrying [tool.astro], valid or not, or a 1.x .astro/config.yaml. That is project.IsAstroProject, the same test in every directory, ~ included. No walk skips ~, and nothing overrides the answer there.
• A .astro/config.yaml that is the CLI's own settings file is not a 1.x marker. config.IsSettingsFile is the one definition. It compares by path (fileutil.SamePath) against HomeConfigFile, wherever ASTRO_HOME puts it, and against ~/.astro/config.yaml, where the settings live without ASTRO_HOME and where 1.x keeps its own. The second path is there because a manual run showed what happens without it: under ASTRO_HOME, a ~/.astro/config.yaml became a 1.x project and put every directory below ~ inside it. v2 always excluded that file.
• internal/project cannot import config/, so IsAstroProject and HasLegacyConfig take the settings test as an argument. config.IsAstroProject and config.IsProjectDir pass IsSettingsFile. astro init gets it through Deps.IsSettingsFile, which the root wires.

What follows from it:

• A project in ~ is a project. It refuses a dbt project or a non-DAG bundle inside it. It draws astro init sub's nested warning. From ~/dags, the advice names ~.
• ~ with only the settings is no project. It is refused with HomeDirAdvice, as before. APC never takes the settings for a 1.x project.
• ~ with a manifest that fails validation passes the check, as anywhere. astro deploy then reports the validation error.
• ASTRO_HOME=/x: /x is not a project, and nothing below it is inside one because of the settings.
• astro init in ~ refuses only to make a new project there. A ~ that is already a project goes on to init, which answers as it does for any project (ErrAlreadyAstroProject for a manifest).

Also in this round:

1. An existing pyproject.toml that cannot be read (EACCES, EISDIR, any load error that is not a ParseError) is returned as the read error. It no longer gets "run astro init" advice. Every error from the project test is now reported as itself, so verifyFailed is gone.
2. A subdirectory whose own pyproject.toml does not parse, inside a project, is told "This directory is inside the project at : run this from there" first, with the parse error after it (still errors.As-able). With no enclosing project, the parse error is returned as before.
3. Each walk asks one question per level, the project test. The home and settings test runs only when a .astro/config.yaml exists.

Removed: fileutil.IsHomeDir, fileutil.SamePathAs, the walks' skip argument, config's home-stat cache, manifestLoads, verifyFailed, and NewDeps' IsHomeDir default. The root wires IsHomeDir and IsSettingsFile; nil means none is known. Non-test code is 44 lines shorter than at bb16383.

Superseded by this round: round 1 item 2 (in part) and item 3's "~ never counts", round 2 items D, E (the ~ skip) and I (skip), and round 3 items 4, 5 and 7.

Tests:

• A ~ manifest project: dbt and non-DAG bundle containment refused, astro init sub from ~ warns, the advice from ~/dags names ~.
• ~ with only the settings: refused with HomeDirAdvice, by both checks.
• ~ with an invalid manifest: the check passes and astro deploy returns the *manifest.ValidationError.
• astro init in ~: creating is refused, a re-run reaches init.
• Settings under ASTRO_HOME, run through the real initHome, also through a symlink: neither that directory nor ~/.astro/config.yaml is a project.
• An EISDIR or EACCES pyproject.toml gives the read error.
• A subdirectory with a ParseError inside a project gives the enclosing advice first.

Mutation checks, where each test failed against the mutated code:

• HasLegacyConfig ignoring the settings test: config, utils, init and project tests failed.
• IsSettingsFile built from HomePath instead of HomeConfigFile: the ASTRO_HOME cases failed.
• IsSettingsFile without the ~/.astro/config.yaml path: the ASTRO_HOME home cases in config and utils failed.
• astro init's re-run allowance removed: TestInitReRunsInAHomeDirectoryProject failed.

First round (c380e09)

1. APC astro deploy --dags refused a pyproject.toml project. A DAG-only deploy uploads dags/ from the working directory and builds nothing. (Reworked in round 2, item A.)
2. In $HOME the advice was to run astro init. Both checks now give a home-directory message (its .astro/config.yaml holds the CLI's settings; change to a project directory). astro init refuses the home directory with a usage error (exit 2, kind usage under -o json, nothing written). astro init my-project run from ~ still works. The new code is in cmd/local/initguard.go. initcmd.go gains only two calls, to stay clear of Refuse to convert a 1.x project under an APC context, and give the same advice everywhere #2285.
3. In a subdirectory of a project the advice was to make a second, nested project. The error now says "This directory is inside the project at : run this from there". ~ never counts as the enclosing project. astro init inside a project now warns on stderr and still creates the project. I chose a warning over a refusal because a repository can legitimately hold several projects, and commands always find the nearest manifest, so the outer project keeps working from its own root. What changes is which project a command run below the new one acts on, and that is what the warning says.
4. astro remote deploy --image-name still checked for a project. DeployClientImage with an image name tags and pushes it without building anything. The check is now skipped when an image is named. (The test is on the value now: round 2, item B.)
5. The manifest bypass moved into utils.EnsureProjectDir. That makes its "run astro init" advice true by construction. The copies in cmd/astro/deploy.go and cmd/astro/remote.go are gone; --image-name, --non-dags and --dags-path keep their own bypasses.
6. EnsureDockerfileProjectDir did not check for a Dockerfile. It now checks <WorkingPath>/Dockerfile, the fixed path deploy.Airflow parses. A .astro/config.yaml with no Dockerfile gets " has a .astro/config.yaml but no Dockerfile…" instead of a later "failed to parse dockerfile".
7. The stat-error branch is tested for the APC variant (a NUL-byte WorkingPath).
8. The APC advice now names a path users can follow: run astro package --tag <image>, then astro deploy --image-name <image>.
9. The messages are named consts in cmd/utils (AstroProjectDirAdvice, APCProjectDirAdvice, APCNoDockerfileAdvice, HomeDirAdvice, EnclosingProjectAdvice). The tests assert against those consts plus a few key tokens.

Second round (7e6783a, 105dec5)

A. Regression: APC astro deploy --dags refused a 1.x project with no Dockerfile. The Dockerfile check ran for DAG-only deploys too. The cmd/apc var is back to its v2 name, EnsureProjectDir, and is now ensureDeployProjectDir. That function runs utils.EnsureProjectDir for --dags, which accepts a manifest or a .astro/config.yaml, and utils.EnsureDockerfileProjectDir for everything else. The rename in round 1 is undone, so deployment_json_test.go matches v2 again.
B. An empty --image-name= skipped the project check and then built from a directory that is not a project. astro remote deploy and APC astro deploy now test the bound value (remoteImageName != "", imageName != "") instead of Changed.
C. A manifest project whose .astro is a file (ENOTDIR) or unreadable was refused. project.IsAstroProject reads the manifest first. A pyproject.toml carrying [tool.astro], valid or not, makes the directory a project whatever state .astro is in.
D. ~ passed when ~/pyproject.toml had [tool.astro], or could not be read. HasManifest counts read and parse errors as a manifest. The home check now runs before anything in the directory is read, so ~ is always refused with HomeDirAdvice. (Narrowed in round 3, item 4: a manifest that loads makes ~ a project.)
E. False nested warnings. A malformed ~/pyproject.toml or an unreadable ancestor used to produce one. The walk now skips ~ and the filesystem root. The predicate is strict: a pyproject that cannot be read or parsed is an error, not a project, and a walk treats that as "not a project".
F/G. One home test. config.IsHomeDir(path) compares against config.HomePath with os.SameFile. When either path cannot be stat'ed, it falls back to comparing the symlink-resolved absolute paths, case-insensitively on Windows. IsProjectDir and both Ensure* checks use it, and so does astro init through a new Deps.IsHomeDir that cmd/root.go wires to config.IsHomeDir (cmd/local cannot import config/). os.UserHomeDir is no longer used.
H. One definition of "the project above". project.IsAstroProject (a manifest or a .astro/config.yaml) is used by EnsureProjectDir, its enclosing advice and astro init's nested warning. Init and deploy now agree:

• A parent with a config.yaml and no Dockerfile is named by both.
• A parent with a Dockerfile and a bare .astro is named by neither.

APC's advice uses its own Dockerfile-project predicate, consistently.
I. One walk. fileutil.NearestDir(dir, skip, match) checks the directory and then each one above it. It passes over ~ (via skip) and the filesystem root, and returns the first error from match. config.IsWithinProjectDir and cmd/astro's isWithinManifestProject delegate to it. project.Enclosing is deleted. A side effect: IsWithinProjectDir no longer asks about the relative "" / C: first path component, which used to resolve against the working directory.
J. Each rule is written once. EnsureProjectDir and EnsureDockerfileProjectDir are ensureDir(predicate, advice, …), with the same predicate passed to the enclosing-project advice.

Tests cover each item. Two of them were mutation-checked, and the tests failed when the fix was reverted:

• A: ensureDeployProjectDir changed to always run the Dockerfile check.
• D: the home check removed.

config.IsHomeDir was also reverted to a string compare, and the symlink tests failed.

Third round (bb16383)

1. A manifest that does not parse was refused with "run astro init". IsAstroProject returns the parse error for it, and the check wrapped that in the init advice before deploy() could report it. ensureDir now returns a *manifest.ParseError as it is: astro deploy and astro remote deploy both show parse <dir>/pyproject.toml: …, syntax and type errors alike. (A type error inside [tool.astro] is a validation error, which already counted as a project and reaches the manifest deploy's own report.)
2. astro deploy tested Changed(--image-name). It now tests imageName != "" like the other two, so --image-name= runs the project check.
3. dbt and non-DAG bundle containment used the lenient HasManifest. isWithinManifestProject now walks with project.IsAstroProject, so a repository-root pyproject.toml that does not parse (and so shows no [tool.astro]) no longer refuses a dbt project below it.
4. $HOME was refused even with a valid manifest (a container with WORKDIR=$HOME). EnsureProjectDir accepts ~ when ~/pyproject.toml loads (manifest.Load succeeds), and refuses it with HomeDirAdvice otherwise: no project, a manifest that fails to parse or validate, or only ~/.astro/config.yaml. APC's Dockerfile check never accepts ~. astro init still refuses to scaffold into ~. HomeDirAdvice now reads "This is your home directory, and its .astro/config.yaml holds the CLI's own settings, not a project's. Change to a project directory".
5. A nil Deps.IsHomeDir let ~/.astro/config.yaml count as a project. The 1.x marker check itself now ignores the home directory's .astro/config.yaml, so no caller can get this wrong. NewDeps also sets IsHomeDir (to fileutil.IsHomeDir), and the root still replaces it with config.IsHomeDir.
6. The 1.x marker was spelled twice. project.HasLegacyConfig is now the only check for .astro/config.yaml (LegacyConfigDir/LegacyConfigFile). config.IsProjectDir and config.IsWithinProjectDir call it. config/ may import internal/project; archlint forbids only the reverse. One side effect: a .astro that is a file (ENOTDIR) is now "not a project" in config.IsProjectDir too, where it used to be an "error ambiguous".
7. Home stat cost. config.IsHomeDir keeps a fileutil.SamePathAs(HomePath) matcher, rebuilt when HomePath changes, so HomePath is stat'ed once per value. IsWithinProjectDir no longer asks about the home directory twice at each level. HasLegacyConfig asks about the home directory only when the marker is present.
8. The readable adapter is gone. It was duplicated and needed three //nolint:errcheck. fileutil.NearestReadableDir replaces it: a match error means "not this one". The advice walk, the init nested warning and the containment walk use it.

Tests: a syntax error and a type error each surface as *manifest.ParseError, through utils.EnsureProjectDir, astro deploy's PreRunE (with the real check) and astro remote deploy's. astro deploy --image-name= is refused. A dbt project below an unparseable root pyproject deploys. In ~: a manifest that loads passes, including through a symlink; a manifest that fails validation, a syntax error, a typo'd [tool.astro and a lone .astro/config.yaml are each refused, and APC refuses all of them. astro init under a ~ that holds .astro/config.yaml gives no nested warning, both with NewDeps' IsHomeDir and with none. There are also tests for HasLegacyConfig in the home directory, NearestReadableDir and SamePathAs.

Mutation checks, where each test failed against the mutated code:

• 1: the ParseError branch disabled. The utils test, TestDeployProjectCheck and TestRemoteDeployProjectCheck failed.
• 4: the home accept removed. The loads case and the symlink test failed.
• 4: the home accept made lenient (HasManifest/IsAstroProject). Every refuse case failed.
• 3: the containment walk put back on HasManifest. The new dbt test failed.

Checks

These were run at the fifth round (6ce97ec):

• go build ./... and GOOS=windows go build ./..., plus GOOS=windows go vet on the changed packages

• make test

• make test-e2e (tier 0)

• make lint (no autofix rewrites)

• make lint-goos

• make deadcode

• A manual run of the built binary under a temp HOME, XDG_CACHE_HOME and ASTRO_TELEMETRY_DISABLED=1, which:

  • refused deploy and init in ~ with only the settings;
  • warned on astro init sub from a ~ project;
  • reported the validation error for an invalid ~ manifest;
  • named ~ from ~/dags/x.

  The temp HOME sat inside the real one, so a walk that got past it reached the real ~/.astro/config.yaml. Under a temp HOME that file is neither settings path, so it counts as a 1.x project, as it did in v2. The ASTRO_HOME case was therefore checked by the tests, not by the manual run.

Merge with the open PRs (at 6ce97ec)

• Warn when an APC --image-name deploy ships only the image's DAGs #2284 (apc-deploy-warn-no-dags, at ac4f4a0): git merge-tree is clean. The merged tree passes go build ./..., go vet on cmd/..., config, internal/project and pkg/fileutil, and the tests in cmd/utils, cmd/local, cmd/apc, cmd/astro, cmd, config, internal/project and pkg/fileutil.
• Refuse to convert a 1.x project under an APC context, and give the same advice everywhere #2285 (init-keep-apc-dockerfile, at 8be1cc5): git merge-tree is clean. The merged tree passes the same build, vet and tests.

[coveralls-official]

Coverage Report for CI Build 37884442998

Coverage increased (+0.07%) to 57.536%

Details

• Coverage increased (+0.07%) from the base build.
• Patch coverage: 13 uncovered changes across 4 files (168 of 181 lines covered, 92.82%).
• 2 coverage regressions across 2 files.

Uncovered Changes

File	Changed	Covered	%
pkg/fileutil/paths.go	48	39	81.25%
cmd/local/initguard.go	19	17	89.47%
cmd/utils/utils.go	45	44	97.78%
internal/project/project.go	31	30	96.77%
Total (10 files)	181	168	92.82%

Coverage Regressions

2 previously-covered lines in 2 files lost coverage.

File	Lines Losing Coverage	Coverage
cmd/utils/utils.go	1	96.88%
internal/project/project.go	1	91.67%

---

Coverage Stats

Relevant Lines:	69171
Covered Lines:	39798
Line Coverage:	57.54%
Coverage Strength:	201.64 hits per line

---

💛 - Coveralls

[schnie]

Replaced by #2290: the core fixes on v2's existing helpers. This branch rewrote project detection and each review found new regressions; the follow-ups listed in #2290 track the rest. Branch kept for reference.