Commit Graph

2 Commits

Author SHA1 Message Date
Pascal THUET
490566847c feat(cli): honor SPECIFY_INIT_DIR in the specify CLI project resolver (#3186)
* feat(cli): honor SPECIFY_INIT_DIR in the specify CLI project resolver

The shell resolver honors SPECIFY_INIT_DIR (#2892), but the Python CLI did
not: it resolved the project as Path.cwd() + a .specify/ check and never read
the override. So setup-plan.sh respected it while `specify integration install`
ignored it, and you still had to cd into the member project.

Route project resolution through a shared _resolve_init_dir_override() that
applies the shell resolver's validation rules (relative to cwd, must exist and
contain .specify/, hard error, no fallback, same error strings). It's wired into
_require_specify_project() — the chokepoint for every project-scoped subcommand
(integration/extension/workflow/preset/...) — and the `workflow run <file>`
standalone path, which re-applies its symlinked-.specify guard on the override
branch too. init is unchanged: it creates .specify/, so the must-pre-exist rule
doesn't apply.

The resolver canonicalizes symlinks via Path.resolve() while the shell keeps the
logical path; they agree for non-symlinked paths (documented in the resolver).

Tests in tests/test_init_dir_cli.py mirror the strict cases from test_init_dir.py
through the CLI; conftest now strips SPECIFY_* for the whole suite so a stray
export can't perturb the now-env-reading resolver. Docs note the CLI applies the
same rules.

Discussion: github/spec-kit#2834

(Disclosure: I used an AI coding agent to audit the call sites and resolver,
draft the change, and run an adversarial code review; reviewed by me.)

* fix(cli): honor SPECIFY_INIT_DIR for bundle commands

Assisted-by: Codex (model: GPT-5, autonomous)

* fix(bundler): refuse symlinked .specify on the SPECIFY_INIT_DIR override path

find_project_root refuses a symlinked .specify (following it could read/write
outside the tree, and a test pins that), but the SPECIFY_INIT_DIR override added
for bundle commands returned early and skipped that guard:
_resolve_init_dir_override validates .specify with is_dir(), which follows
symlinks. So `specify bundle` accepted via the override a layout the cwd path
rejects. Re-check the override result with the same guard, plus a regression test.

(Disclosure: found via an AI code review and fixed with an AI coding agent;
reviewed by me.)

* fix(cli): keep SPECIFY_INIT_DIR strict for bundles

Treat an explicit symlinked SPECIFY_INIT_DIR project as a hard bundle error instead of returning no project, which could initialize the current directory. Align the docs with the actual unset resolver behavior.

Assisted-by: Codex (model: GPT-5, autonomous)

* docs(core): note symlinked .specify handling differs across CLI surfaces

A symlinked .specify is followed by integration/extension/workflow (matching the
shell resolver) but refused by bundle and workflow run <file> (write
confinement). Document the asymmetry so it reads as intentional.

(Disclosure: AI-assisted; reviewed by me.)

* docs(core): reframe symlinked .specify note around the override invariant

Per maintainer feedback on #3186: SPECIFY_INIT_DIR relocates where the project
is, not how a surface treats symlinks. Each surface keeps its cwd-path stance
(write surfaces refuse a symlinked .specify, read/config surfaces follow it),
so the split is one policy relocated, not an inconsistency.

* docs: address Copilot review on resolver docstrings

- _project.py: the error messages "mirror" the shell wording rather than
  "match" it (the CLI renders a Rich `Error:` line, the shell a plain `ERROR:`).
- find_project_root: document that honoring SPECIFY_INIT_DIR when start is None
  can raise typer.Exit / BundlerError, so the Path | None signature isn't
  surprising to direct callers.

* docs(bundler): note require_project_root inherits the override raise behavior

find_project_root can raise typer.Exit / BundlerError under the SPECIFY_INIT_DIR
override (start=None); require_project_root inherits that, so document it
alongside its own BundlerError-on-missing-project.

* docs: clarify symlinked project root behavior

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

* Address SPECIFY_INIT_DIR review feedback

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)

* Route workflow JSON errors to stderr

Assisted-by: OpenAI Codex (model: GPT-5, autonomous)
2026-07-01 15:55:18 -05:00
Pascal THUET
9cd20c6c25 feat(dev): add integration scaffolder (#2685)
* feat(dev): add integration scaffolder

* fix(dev): address integration scaffold review feedback

* fix(dev): address scaffold follow-up review

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix(dev): default scaffolded integrations to multi_install_safe = False

The scaffold template emitted `multi_install_safe = True` alongside a
placeholder `context_file = "AGENTS.md"`. Registered as-is, that violates the
registry contract (test_safe_integrations_have_distinct_context_files): codex
already pairs AGENTS.md with multi_install_safe = True, so the generated
boilerplate would collide on first registration.

Default the scaffold to False (matching IntegrationBase) so generated code is
registry-test-friendly out of the box; contributors opt in once they pick a
unique context_file. Aligns the generated test skeleton and both scaffold
tests, which previously contradicted each other (one expected True, one False).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(dev): harden scaffold writes and accept case-insensitive --type

- Guard scaffold_integration() against symlinked target directories: walk
  each path component under the repo root and refuse symlinked dirs, then
  confirm the write destination resolves inside the repo (mirrors the
  manifest directory guard). Prevents scaffolding outside the repo when a
  contributor's integrations/tests path is symlinked.
- Make the `--type` click.Choice case-insensitive so `--type YAML` is
  accepted, matching scaffold_integration()'s strip()/lower() normalization
  instead of rejecting at the CLI layer.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(dev): report scaffold filesystem failures as a clean CLI error

The `dev integration scaffold` command only caught FileExistsError/ValueError,
so an OSError raised during mkdir()/write_text() (permission denied, read-only
checkout, a path component that is a file, ...) bubbled up as a traceback
instead of a clean error + exit code. Broaden the handler to OSError (which
also covers FileExistsError) and add coverage for the filesystem-error path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(dev): move scaffold command under integration

* fix(dev): roll back partial scaffold writes

* fix(dev): correct lint docs and generated test docstring

- local-development.md: ruff check src/ is enforced in CI, not absent
- scaffolded test docstring: drop misleading 'scaffold' wording

* fix(scaffold): create only leaf integration directory

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-16 17:48:40 -05:00