diff --git a/AGENTS.md b/AGENTS.md index 7ab56a8edc..3742fdeb34 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -246,8 +246,13 @@ context_markers: ## Team Directives & Constitution This project is bound by the team-ai-directives knowledge base. -- **Strict Compliance**: You MUST check if a skill applies BEFORE responding to any task or question. If a skill applies, you MUST invoke it immediately. -- **Team Constitution**: Read and strictly adhere to the principles in `../agentic-sdlc-team-ai-directives/context_modules/constitution.md`. When creating or updating the project's constitution, you MUST inherit and align with these principles. +- **Strict Compliance**: You MUST invoke the `team-boot` skill BEFORE responding to any task or question. This is not optional and applies to every interaction. If a skill applies, you MUST invoke it immediately. +- **Anti-Patterns** (do NOT rationalize skipping the skill check): + - "Let me explore the codebase first" -> Skills tell you HOW to explore. Check first. + - "This is a plain message, not a spec command" -> Skill check applies to EVERY interaction. + - "I'll check after understanding the request" -> Check BEFORE understanding -- skills guide your questions. + - "This is too simple" -> Simple things become complex. Check first. +- **Team Constitution**: Read and strictly adhere to the principles in `./agentic-sdlc-team-ai-directives//context_modules/constitution.md`. When creating or updating the project's constitution, you MUST inherit and align with these principles. - **Rules, Personas & Examples**: Use the `team-discover` skill to find and load relevant rules, personas, or examples before making changes. For additional context about technologies to be used, project structure, diff --git a/CHANGELOG.md b/CHANGELOG.md index 41dc3e9d9e..bca9844da6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ All notable changes to the Specify CLI and templates are documented here. +# [0.16.0+adlc2] - 2026-08-07 + +### Changed + +- **levelup extension no longer preinstalled**: `extensions/catalog.json` now + sets `levelup.preinstall` to `false`. The `levelup` extension's commands + (`/levelup.init`, `/levelup.clarify`, `/levelup.specify`, `/levelup.skill`, + `/levelup.implement`, `/levelup.validate`) overlap with the + `adlc-team-skills` `levelup-*` skills/commands. Removing levelup from + auto-install eliminates the command redundancy when both toolkits are used + together (the recommended coexistence flow: install `adlc-team-skills` first, + run `team-setup`, then `specify init` without `--team-ai-directives`). + The extension remains bundled and installable on demand via + `specify extension install levelup`. + # [0.15.2+adlc4] - 2026-08-05 ### Fixed @@ -4299,6 +4314,31 @@ This release migrates fork-specific customizations to a preset system to reduce The following entries are from the upstream spec-kit project and are included for reference. +## [0.16.0] - 2026-08-05 + +### Changed + +- fix: keep long frontmatter values on a single line (#3989) +- fix: skip corrupted run state files in list_runs (#3817) +- fix: skip corrupted run state files in list_runs (#3814) +- Add July 2026 newsletter (#3987) +- fix(presets): start fresh on a non-UTF-8 preset registry (#3955) +- docs: clarify agent PR review prioritization (#3985) +- fix(events): preserve a non-UTF-8 config.toml on hook install/teardown (#3963) +- fix(extensions): treat an unreadable staged backup as a conflict (#3962) +- fix(manifests): reject non-string requires.speckit_version (#3980) +- fix(extensions): reject reinstall when a kept config cannot be read (#3960) +- [extension] Update Charter extension to v0.5.1 (#3983) +- fix(events): return None for an unparseable script command (#3957) +- feat(events): context injection for opencode and JSON-envelope agent hooks (#3934) +- Add TDD Extension to community catalog (#3982) +- Update Archive Extension to v1.1.0 (#3981) +- feat(copilot): default integration to skills (#3976) +- fix(events): ignore non-UTF-8 event overrides (#3897) +- fix: cap stdin read at 1 MiB to prevent DoS (#3857) +- fix(workflows): reject mismatched run state IDs (#3899) +- chore: release 0.15.2, begin 0.15.3.dev0 development (#3953) + ## [0.15.2] - 2026-08-03 ### Changed diff --git a/FORK.md b/FORK.md index a583661a71..815db8bb06 100644 --- a/FORK.md +++ b/FORK.md @@ -72,6 +72,7 @@ When a fork release changes only bundled extension behavior, keep the CLI versio | Version | Date | Base Upstream | Changes | |---------|------|---------------|---------| +| 0.16.0+adlc1 | 2026-08-06 | 0.16.0 (`adb2413a`) | Upstream merge (18 commits, post-0.15.2 → 0.16.0 release `6fa8c9aa`). New upstream feature: `feat(init): scaffold managed .specify/.gitignore` (#4000 — manifest-tracked `.specify/.gitignore` excludes `feature.json` + `extensions/*/local-config.yml`; routed through shared-infra overwrite/skip/preserve policy; 8 integration test inventories + new `test_shared_infra_gitignore.py`). New upstream fixes: escape user-supplied values in `specify init` output (#3787 — `_escape_markup` + `_shell_quote_arg` for the `cd` line, the only CLI module without escaping), reapply presets/extensions on `init --here --force` (#3995 — `_register_presets_for_agent`/`_register_extensions_for_agent` after `manifest.save()`), bound response read in integration catalog fetch (#3812 — `read_response_limited` + `MAX_JSON_METADATA_BYTES`), `missing_ok` temp-file cleanup (#3803), unreadable run state in `workflow status` (#3999), skip corrupted run state in `list_runs` (#3814/#3817), non-UTF-8 extension registry (#3998) + unreadable layer in `resolve_content` (#3959) + EOFError wrap for truncated tar.gz (#3938), line-anchored `---` delimiter scan for hermes+kimi (#3739), keep long frontmatter on single line (#3989 — `yaml.dump(width=float("inf"))`), legacy code-page fix for `check_prerequisites`/`setup_tasks` Python scripts (#3890/#3892). **4 conflicts resolved**: `pyproject.toml` (kept fork name/description, version → `0.16.0+adlc1`), `commands/init.py` (wrapped user-controlled display values with `_escape_markup` *inside* fork's `accent()` theming — project name, paths, echoed `--integration`/`--script`, agent folder in gitignore hint; adopted `_shell_quote_arg` for the Next Steps `cd` line; `f31b2b45` reapply-on-`--force` block auto-merged in un-themed region), `tests/integrations/test_integration_catalog.py` (adopted upstream's restructured file + bounded `FakeResponse`; **fixed mock target**: upstream's `_patch_urlopen` patched `urllib.request.urlopen` but `open_url` uses `opener.open()` which never calls module-level `urlopen` → tests hit real network; switched mock to `open_url` directly, preserving upstream's bounded-read `FakeResponse` for `read_response_limited` contract; applies to both `_patch_urlopen` and the oversized-response regression test), `tests/integrations/test_integration_base_toml.py` (added `.specify/.gitignore` to fork's `stem_pfx` inventory). All other conflict-candidate files auto-merged cleanly, preserving fork customizations: `shared_infra.py` (`.gitignore` block + `missing_ok` + `COMMAND_PREFIX`/`project_path`/theming), `extensions/__init__.py` (non-UTF-8 registry + catalog-URL override + alias logic), `hermes/__init__.py` (line-anchored delimiter + `resolve_command_alias`/`COMMAND_PREFIX`), `agents.py` (`width=float("inf")` + `_skip_primary`/`inject_model_invocation_flag`), `presets/__init__.py` (`resolve_content` guard + `_cleanup_replaced_commands`), `workflows/_commands.py` (unreadable-run-state guard + theming), `update_agent_context.py` (symlink-safe recursive plan discovery + `missing_ok` + fork team-directives block). Fork modules (`_*_fork.py`, `extensions_fork.py`) untouched. No `templates/` changes upstream → no preset porting. Ruff clean (`ruff@0.15.0`). 2629 tests pass across merge-affected files (110 catalog, 122 init/cli, 138 generic/cline/copilot, 2259 infra/workflow/preset/extension/parity). Live smoke: `specify init` scaffolds `.specify/.gitignore` (manifest-tracked, excludes `feature.json` + `extensions/*/local-config.yml`); `specify extension update` (after clearing `.specify/extensions/.cache/`) finds all fork-bundled extensions up-to-date via fork-repo catalog URL. | | 0.15.2+adlc4 | 2026-08-05 | 0.15.2 (`68daed8f`) | Extension catalog URL override. `ExtensionCatalog.DEFAULT_CATALOG_URL` and `COMMUNITY_CATALOG_URL` now point at the fork's repo (`tikalk/agentic-sdlc-spec-kit`) instead of upstream (`github/spec-kit`). Previously `specify extension update` fetched upstream's catalog → fork-bundled extensions (`levelup`, `team-ai-directives`, `evals`, `edd`, `architect`) showed "Not found in catalog" and `tdd`/`product` showed "Updates not allowed from 'community'" (found in upstream's community catalog instead of the fork's bundled catalog). Fork constants `FORK_DEFAULT_CATALOG_URL`/`FORK_COMMUNITY_CATALOG_URL` added to `_core_fork.py`; override applied in `extensions/__init__.py` ExtensionCatalog class body (try/except with fallback to upstream URLs). 106 catalog tests pass. Live smoke: `specify extension update` now finds all 8 fork-bundled extensions as up-to-date. Note: stale catalog cache (from old upstream URL) may persist for up to 1 hour after upgrade; clear `.specify/extensions/.cache/` to force immediate refresh. | | 0.15.2+adlc3 | 2026-08-05 | 0.15.2 (`68daed8f`) | assess/bug extension command-reference fix. `_build_preset_command_placeholder_map()` in `integrations/base.py` extended to scan `.specify/extensions/*/extension.yml` `provides.commands[].aliases` (previously only scanned preset manifests) — `__SPECKIT_COMMAND_ASSESS_*__`/`__SPECKIT_COMMAND_BUG_*__` placeholders now resolve to `/assess.*`/`/bug.*` instead of the broken prefix fallback `/spec.assess.*`. Aliases added to `assess` extension (1.0.0→1.0.1, 5 aliases: `assess.intake/research/define/shape/decide`) and `bug` extension (1.0.0→1.0.1, 3 aliases: `bug.assess/fix/test`); catalog.json versions bumped. With `EXTENSION_ALIAS_PATTERN_ENABLED`, files install as `assess.intake.md`/`bug.fix.md` (fork convention, matching git extension). Latent defect since `0.14.4+adlc1` (when assess/bug were bundled). Regression tests in `test_base.py` (placeholder map unit), `test_assess_extension.py` + `test_bug_extension.py` (alias declaration + rendering: no unresolved placeholders, refs match installed files). Existing `test_extension_command_dot/hyphen` tests updated to use clean tmp_path (no extensions) for fallback path. Ruff clean (`ruff@0.15.0`). 1189 tests pass. Live smoke: `specify init` + `extension add assess` → `.opencode/commands/assess.intake.md` with `/assess.research` refs and `/spec.specify` handoff. | | 0.15.2+adlc2 | 2026-08-05 | 0.15.2 (`03d71b33`) | Upstream merge (13 commits, post-0.15.2, no new release tag). New upstream features: `feat(copilot): default integration to skills` (#3976 — Copilot defaults to skills layout, `--commands` opts back to `.agent.md`+`.prompt.md`; fork adapted `is_skills_mode()` to check both `spec-`/`speckit-` prefixes, re-applied 4 fork customizations onto upstream's rewritten module, fixed `build_command_invocation()` bare-name canonicalization); `feat(events): context injection for opencode and JSON-envelope agent hooks` (#3934, authored by fork maintainer upstream — auto-merged clean). Upstream fixes: non-UTF-8 preset registry (#3955), non-UTF-8 config.toml on hook install/teardown (#3963), unreadable staged backup as conflict (#3962), non-string requires.speckit_version (#3980), reinstall when kept config unreadable (#3960), None for unparseable script command (#3957), migration target-options validation. Community catalog: TDD extension (#3982 — fork has its own bundled tdd), Charter v0.5.1 (#3983), Archive v1.1.0 (#3981). **5 conflicts resolved**: `AGENTS.md` (adopted upstream's "Optional overrides" section + "Opening pull requests" PR-prioritization; preserved fork header/SPECKIT markers), `copilot/__init__.py` (re-applied fork customizations + prefix-aware `is_skills_mode()`), `test_cli.py` (skills-default assertions adapted to fork naming), `test_integration_copilot.py` (alias-aware `build_command_invocation` test assertions), `test_integration_subcommand.py` (copilot switch/upgrade tests adapted). All semantic hotspots (`events.py`, `extensions/__init__.py`, `presets/__init__.py`, `base.py`, `_migrate_commands.py`, `integration_runtime.py`) auto-merged cleanly; fork modules untouched. No `templates/` changes → no preset porting. Ruff clean (`ruff@0.15.0`). 1375+ tests pass across merge-affected files. | diff --git a/docs/reference/core.md b/docs/reference/core.md index 3318264b4f..fdf0b80e7f 100644 --- a/docs/reference/core.md +++ b/docs/reference/core.md @@ -57,6 +57,8 @@ specify init my-project --integration copilot --preset compliance > **Two resolution axes.** `SPECIFY_INIT_DIR` selects the **project** (which directory contains `.specify/`); `SPECIFY_FEATURE_DIRECTORY` / `.specify/feature.json` select the **feature** within that project. They are independent — project first, then feature. +> **Version control.** `specify init` scaffolds a managed `.specify/.gitignore` that excludes machine-local state — `feature.json` (the current-feature pointer, rewritten on every feature switch) and per-machine extension `extensions/*/local-config.yml` overrides — while leaving everything else under `.specify/` (constitution, templates, scripts, extension config) shareable so teams stay aligned. Like the rest of `.specify/`'s shared scripts and templates, the file is tracked in the shared-infrastructure manifest: your edits are preserved on re-init and `specify init --here --force` restores the managed content. It is intentionally left in place by `specify integration uninstall`, which only removes the uninstalled agent's own files. + > **Symlinked project roots.** `SPECIFY_INIT_DIR` relocates *where* the project is, not *how* a command treats symlinks: each command keeps its existing cwd-path stance. Commands that traverse and write project files through broad input paths (`bundle`, `workflow run `) refuse a symlinked `.specify/` to preserve write confinement. Other project-scoped commands keep their existing behavior when `SPECIFY_INIT_DIR` points at a project root, which may include following a symlinked `.specify/`. ## Check Installed Tools diff --git a/extensions/agent-context/scripts/python/update_agent_context.py b/extensions/agent-context/scripts/python/update_agent_context.py index 2c3eb20ac1..dcccb5c709 100644 --- a/extensions/agent-context/scripts/python/update_agent_context.py +++ b/extensions/agent-context/scripts/python/update_agent_context.py @@ -11,8 +11,8 @@ When ``plan_path`` is omitted, the script derives it from ``.specify/feature.json`` (written by /speckit-specify). Falls back to the most -recently modified ``plan.md`` anywhere under ``specs/`` (including nested scoped -layouts such as ``specs///plan.md``) only when feature.json is +recently modified ``plan.md`` found anywhere under ``specs/`` — scoped layouts +nest it as ``specs///plan.md`` — only when feature.json is absent or its plan does not exist yet. """ @@ -173,16 +173,31 @@ def _resolve_plan_path(project_root: str) -> str: if not plan_path: root = Path(project_root).resolve() - plans = sorted( - (root / "specs").rglob("plan.md"), - key=lambda p: p.stat().st_mtime, - reverse=True, - ) - if plans: + specs = root / "specs" + + def _resolved_rel(p: Path) -> Path | None: + # Resolve symlinks before checking containment: relative_to() is + # lexical and would otherwise accept a plan reached through a specs/ + # symlink that points outside the project, emitting an + # in-project-looking path for an out-of-project file (or picking it + # as "most recent"). try: - plan_path = plans[0].relative_to(root).as_posix() - except ValueError: - plan_path = "" + return p.resolve().relative_to(root) + except (OSError, ValueError): + return None + + # Recurse (rather than the old one-level specs/*/plan.md glob) so scoped + # layouts created via SPECIFY_FEATURE_DIRECTORY, e.g. + # specs///plan.md, are still discovered when + # feature.json is absent (#3024). Mirrors the bash and PowerShell twins. + candidates = [] + for p in specs.rglob("plan.md"): + rel = _resolved_rel(p) + if rel is not None: + candidates.append((p, rel)) + candidates.sort(key=lambda pr: pr[0].stat().st_mtime, reverse=True) + if candidates: + plan_path = candidates[0][1].as_posix() return plan_path diff --git a/extensions/catalog.json b/extensions/catalog.json index d655bb3f04..1b76af4d1d 100644 --- a/extensions/catalog.json +++ b/extensions/catalog.json @@ -163,7 +163,7 @@ "name": "LevelUp - Team AI Directives Contributor", "version": "1.0.0", "bundled": true, - "preinstall": true, + "preinstall": false, "description": "Discover and contribute context modules (rules, personas, examples, skills) to team-ai-directives using Context Directive Records (CDRs)", "author": "Agentic SDLC Team", "repository": "https://github.com/tikalk/agentic-sdlc-spec-kit", diff --git a/pyproject.toml b/pyproject.toml index 0fa1998f1f..36ae95d9a9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "agentic-sdlc-specify-cli" -version = "0.15.2+adlc4" +version = "0.16.0+adlc2" description = "Specify CLI (tikalk fork). Agentic SDLC toolkit for Spec-Driven Development with pre-installed extensions and AI integrations." readme = "README.md" requires-python = ">=3.11" diff --git a/scripts/python/check_prerequisites.py b/scripts/python/check_prerequisites.py index 50c31cb513..e909ffb507 100644 --- a/scripts/python/check_prerequisites.py +++ b/scripts/python/check_prerequisites.py @@ -130,14 +130,31 @@ def _print_paths_only(paths: FeaturePaths, json_mode: bool) -> None: print(f"TASKS: {paths.tasks}") +def _status_marker(ok: bool) -> str: + """Return the status glyph, downgraded to ASCII when stdout cannot encode it. + + On Windows sys.stdout falls back to the ANSI code page whenever it is not a + console - a pipe or a file redirect, which is how agents and workflow steps + invoke these scripts - and U+2713 is unencodable in cp1252, so printing it + raised UnicodeEncodeError and aborted the report right after + "AVAILABLE_DOCS:". "[OK]"/"[FAIL]" is the ASCII rendering these markers + already have in-tree: see Test-FileExists in scripts/powershell/common.ps1 + and normalize_status_text in tests/parity_helpers.py. + """ + glyph = "✓" if ok else "✗" + try: + glyph.encode(getattr(sys.stdout, "encoding", None) or "utf-8") + except (LookupError, UnicodeEncodeError): + return "[OK]" if ok else "[FAIL]" + return glyph + + def _check_file(path: Path, description: str) -> None: - marker = "✓" if path.is_file() else "✗" - print(f" {marker} {description}") + print(f" {_status_marker(path.is_file())} {description}") def _check_dir(path: Path, description: str) -> None: - marker = "✓" if _dir_has_entries(path) else "✗" - print(f" {marker} {description}") + print(f" {_status_marker(_dir_has_entries(path))} {description}") def _print_text_results(paths: FeaturePaths, include_tasks: bool) -> None: diff --git a/scripts/python/setup_tasks.py b/scripts/python/setup_tasks.py index b3abb6dc1a..21b0018620 100644 --- a/scripts/python/setup_tasks.py +++ b/scripts/python/setup_tasks.py @@ -55,14 +55,31 @@ def _available_docs(paths: FeaturePaths) -> list[str]: return docs +def _status_marker(ok: bool) -> str: + """Return the status glyph, downgraded to ASCII when stdout cannot encode it. + + On Windows sys.stdout falls back to the ANSI code page whenever it is not a + console - a pipe or a file redirect, which is how agents and workflow steps + invoke these scripts - and U+2713 is unencodable in cp1252, so printing it + raised UnicodeEncodeError and aborted the report mid-listing. + "[OK]"/"[FAIL]" is the ASCII rendering these markers already have in-tree: + see Test-FileExists in scripts/powershell/common.ps1 and + normalize_status_text in tests/parity_helpers.py. + """ + glyph = "✓" if ok else "✗" + try: + glyph.encode(getattr(sys.stdout, "encoding", None) or "utf-8") + except (LookupError, UnicodeEncodeError): + return "[OK]" if ok else "[FAIL]" + return glyph + + def _check_file(path: Path, description: str) -> None: - marker = "✓" if path.is_file() else "✗" - print(f" {marker} {description}") + print(f" {_status_marker(path.is_file())} {description}") def _check_dir(path: Path, description: str) -> None: - marker = "✓" if _dir_has_entries(path) else "✗" - print(f" {marker} {description}") + print(f" {_status_marker(_dir_has_entries(path))} {description}") def main(argv: list[str] | None = None) -> int: diff --git a/src/specify_cli/_download_security.py b/src/specify_cli/_download_security.py index 9d2d95ea72..5ff460666e 100644 --- a/src/specify_cli/_download_security.py +++ b/src/specify_cli/_download_security.py @@ -10,6 +10,7 @@ import tarfile import unicodedata import zipfile +import zlib from collections.abc import Iterator from contextlib import ExitStack, contextmanager from ipaddress import IPv4Address, IPv6Address, ip_address @@ -69,6 +70,19 @@ _BOUNDED_ZIP_COMPRESSION_METHODS = frozenset( (zipfile.ZIP_STORED, zipfile.ZIP_DEFLATED) ) +#: Decompression failures a truncated or corrupt gzip stream raises from +#: ``tarfile``. Most are wrapped in ``TarError``, but two escape raw, and +#: neither derives from ``TarError`` or ``OSError``, so both bypass a +#: ``(TarError, OSError)`` handler: +#: +#: * ``EOFError`` -- from the gzip layer when the stream ends before its +#: end-of-stream marker, i.e. a truncated archive. +#: * ``zlib.error`` -- from a corrupt deflate block. ``tarfile`` converts this +#: to ``ReadError`` while reading a member *header*, but the forward seek it +#: performs to skip member *data* sits outside that conversion, so a corrupt +#: region past the first header escapes raw. +_TAR_DECOMPRESSION_ERRORS = (tarfile.TarError, EOFError, zlib.error) + _ARCHIVE_CONTENT_TYPES: dict[str, ArchiveFormat] = { "application/gzip": "tar.gz", "application/x-gzip": "tar.gz", @@ -166,7 +180,11 @@ def detect_archive_format( try: with tarfile.open(fileobj=archive_file, mode="r:gz"): is_tar_gz = True - except tarfile.TarError: + except _TAR_DECOMPRESSION_ERRORS: + # A truncated gzip stream raises a bare EOFError here rather + # than a TarError, so catching only TarError let it escape + # this probe as a raw exception instead of leaving + # ``is_tar_gz`` False and reporting the format mismatch. pass archive_file.seek(0) except OSError as exc: @@ -1077,7 +1095,7 @@ def safe_extract_tar( mode="r:gz", fileobj=archive_file, ) - except (tarfile.TarError, OSError) as exc: + except (*_TAR_DECOMPRESSION_ERRORS, OSError) as exc: _raise_from(error_type, f"Invalid tar.gz archive: {archive_path}", exc) with archive: @@ -1149,7 +1167,7 @@ def safe_extract_tar( f"of {max_total_bytes} bytes", ) validated.append((member, normalized_name, is_dir)) - except (tarfile.TarError, OSError) as exc: + except (*_TAR_DECOMPRESSION_ERRORS, OSError) as exc: _raise_from( error_type, f"Invalid tar.gz archive: {archive_path}", diff --git a/src/specify_cli/_utils.py b/src/specify_cli/_utils.py index 48db258b98..c644ee8856 100644 --- a/src/specify_cli/_utils.py +++ b/src/specify_cli/_utils.py @@ -206,8 +206,8 @@ def atomic_write_json(target_file: Path, payload: dict[str, Any]) -> None: os.replace(temp_path, target_file) except Exception: - if temp_path and temp_path.exists(): - temp_path.unlink() + if temp_path: + temp_path.unlink(missing_ok=True) raise try: diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 6b06269467..6afa0db937 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -157,7 +157,11 @@ def render_frontmatter(fm: dict) -> str: return "" yaml_str = yaml.dump( - fm, default_flow_style=False, sort_keys=False, allow_unicode=True + fm, + default_flow_style=False, + sort_keys=False, + allow_unicode=True, + width=float("inf"), ) return f"---\n{yaml_str}---\n" diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index c04b803c86..1783963199 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -3,13 +3,16 @@ from __future__ import annotations import os +import shlex import shutil +import subprocess import sys from pathlib import Path from typing import Any import typer from rich.live import Live +from rich.markup import escape as _escape_markup from rich.panel import Panel from .._agent_config import ( @@ -204,6 +207,25 @@ def _install_extension_during_init(project_path: Path, ext_spec: str, speckit_ve return f"{manifest.name} v{manifest.version} installed" +def _shell_quote_arg(value: str) -> str: + """Quote *value* as one argument for the shells of the host OS. + + The Next Steps ``cd`` line is copy-pasted into whichever shell ran + ``specify init``, so it is quoted for the host the same way + ``_version._render_argv`` renders its copy-pasteable installer command: + ``list2cmdline`` on Windows, ``shlex.quote`` elsewhere. The Windows branch + must emit double quotes -- ``cd 'my project'`` is a path-not-found in + cmd.exe, while ``cd "my project"`` is accepted by cmd.exe, PowerShell and + Git Bash alike. A value needing no quoting is returned unchanged. + + Whitespace only. PowerShell also glob-expands ``[``/``]`` and expands + ``$``/backtick inside double quotes, so such a name still needs + ``Set-Location -LiteralPath`` there -- syntax invalid in cmd.exe and sh, so + this shell-neutral line cannot cover it. + """ + return subprocess.list2cmdline([value]) if os.name == "nt" else shlex.quote(value) + + def ensure_constitution_from_template( project_path: Path, tracker: StepTracker | None = None ) -> None: @@ -391,7 +413,10 @@ def init( if integration: resolved_integration = get_integration(integration) if not resolved_integration: - console.print(f"[red]Error:[/red] Unknown integration: '{integration}'") + console.print( + f"[red]Error:[/red] Unknown integration: " + f"'{_escape_markup(str(integration))}'" + ) available = ", ".join(sorted(INTEGRATION_REGISTRY)) console.print(f"[yellow]Available integrations:[/yellow] {available}") raise typer.Exit(1) @@ -467,20 +492,21 @@ def init( project_path = Path(project_name).resolve() dir_existed_before = project_path.exists() if project_path.exists(): + safe_name = _escape_markup(str(project_name)) if not project_path.is_dir(): console.print( - f"[red]Error:[/red] '{project_name}' exists but is not a directory." + f"[red]Error:[/red] '{safe_name}' exists but is not a directory." ) raise typer.Exit(1) existing_items = list(project_path.iterdir()) if force: if existing_items: - console.print(f"[yellow]Warning:[/yellow] Directory '{project_name}' is not empty ({len(existing_items)} items)") + console.print(f"[yellow]Warning:[/yellow] Directory '{safe_name}' is not empty ({len(existing_items)} items)") console.print("[yellow]Template files will be merged with existing content and may overwrite existing files[/yellow]") - console.print(f"{accent('--force supplied:')} merging into existing directory '{accent(project_name)}'") + console.print(f"{accent('--force supplied:')} merging into existing directory '{accent(safe_name)}'") else: error_panel = Panel( - f"Directory already exists: '{accent(project_name)}'\n" + f"Directory already exists: '{accent(safe_name)}'\n" "Please choose a different project name or remove the existing directory.\n" "Use [bold]--force[/bold] to merge into the existing directory.", title="[red]Directory Conflict[/red]", @@ -494,7 +520,7 @@ def init( if integration: if integration not in AGENT_CONFIG: console.print( - f"[red]Error:[/red] Invalid integration '{integration}'. Choose from: {', '.join(AGENT_CONFIG.keys())}" + f"[red]Error:[/red] Invalid integration '{_escape_markup(str(integration))}'. Choose from: {', '.join(AGENT_CONFIG.keys())}" ) raise typer.Exit(1) selected_ai = integration @@ -533,12 +559,14 @@ def init( setup_lines = [ accent("Specify Project Setup", bold=True), "", - f"{'Project':<15} {accent(project_path.name)}", - f"{'Working Path':<15} [dim]{current_dir}[/dim]", + f"{'Project':<15} {accent(_escape_markup(project_path.name))}", + f"{'Working Path':<15} [dim]{_escape_markup(str(current_dir))}[/dim]", ] if not here: - setup_lines.append(f"{'Target Path':<15} [dim]{project_path}[/dim]") + setup_lines.append( + f"{'Target Path':<15} [dim]{_escape_markup(str(project_path))}[/dim]" + ) _border = accent_style() if _FORK else "cyan" console.print(Panel("\n".join(setup_lines), border_style=_border, padding=(1, 2))) @@ -564,7 +592,7 @@ def init( if script_type: if script_type not in SCRIPT_TYPE_CHOICES: console.print( - f"[red]Error:[/red] Invalid script type '{script_type}'. Choose from: {', '.join(SCRIPT_TYPE_CHOICES.keys())}" + f"[red]Error:[/red] Invalid script type '{_escape_markup(str(script_type))}'. Choose from: {', '.join(SCRIPT_TYPE_CHOICES.keys())}" ) raise typer.Exit(1) selected_script = script_type @@ -618,8 +646,6 @@ def init( tracker.add(key, label) if extensions: - from rich.markup import escape as _escape_markup - for i, ext_spec in enumerate(extensions): tracker.add( f"extension-{i}", f"Install extension: {_escape_markup(ext_spec)}" @@ -682,6 +708,30 @@ def init( ) manifest.save() + if force: + from ..integrations._helpers import ( + _register_extensions_for_agent, + _register_presets_for_agent, + ) + + _register_extensions_for_agent( + project_path, + resolved_integration.key, + force=True, + continuing=( + "The project was re-initialized, but installed extensions" + " may need re-registration." + ), + ) + _register_presets_for_agent( + project_path, + resolved_integration.key, + continuing=( + "The project was re-initialized, but installed presets" + " may need re-registration." + ), + ) + integration_settings = _with_integration_setting( {}, resolved_integration.key, @@ -857,8 +907,6 @@ def init( # Install extensions specified via --extension if extensions: - from rich.markup import escape as _escape_markup - from ..extensions._commands import _refresh_events_and_warn speckit_ver = get_speckit_version() @@ -976,7 +1024,7 @@ def init( if agent_folder: security_notice = Panel( f"Some agents may store credentials, auth tokens, or other identifying and private artifacts in the agent folder within your project.\n" - f"Consider adding {accent(agent_folder)} (or parts of it) to {accent('.gitignore')} to prevent accidental credential leakage.", + f"Consider adding {accent(_escape_markup(str(agent_folder)))} (or parts of it) to {accent('.gitignore')} to prevent accidental credential leakage.", title="[yellow]Agent Folder Security[/yellow]", border_style="yellow", padding=(1, 2), @@ -986,7 +1034,9 @@ def init( steps_lines = [] if not here: - steps_lines.append(f"1. Go to the project folder: {accent(f'cd {project_name}')}") + steps_lines.append( + f"1. Go to the project folder: {accent(f'cd {_escape_markup(_shell_quote_arg(str(project_name)))}')}" + ) step_num = 2 else: steps_lines.append("1. You're already in the project directory!") diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index 217c18bbb1..6b6b3f0eb2 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -680,8 +680,13 @@ def _load(self) -> dict: if not isinstance(data.get("extensions"), dict): data["extensions"] = {} return data - except (json.JSONDecodeError, FileNotFoundError): - # Corrupted or missing registry, start fresh + except (json.JSONDecodeError, UnicodeDecodeError, FileNotFoundError): + # Corrupted or missing registry, start fresh. A registry whose + # bytes cannot be decoded as UTF-8 is the same corruption class as + # malformed JSON — only the exception type differs, and it is + # raised by the text-mode read before JSON parsing begins. OSError + # is deliberately not caught: the data may be intact on disk, and + # starting fresh would let a later _save() wipe it. return {"schema_version": self.SCHEMA_VERSION, "extensions": {}} def _save(self): @@ -4392,11 +4397,10 @@ def _sibling_extension_ids(self) -> list[str]: Returns an empty list if the registry is missing or corrupted (fresh project, ad-hoc test harness) so ``_get_env_config`` degrades to its pre-fix behaviour rather than crashing. ``UnicodeError`` is - caught alongside ``OSError`` because ``ExtensionRegistry._load()`` - opens the file in text mode and only handles ``JSONDecodeError`` / - ``FileNotFoundError``, so a registry file with non-UTF-8 bytes would - otherwise surface a ``UnicodeDecodeError`` here and break *every* - config read instead of degrading gracefully. + kept alongside ``OSError`` as belt-and-braces: ``_load()`` now starts + fresh on non-UTF-8 registry bytes itself, but catching it here too + keeps this call site degrading gracefully rather than breaking *every* + config read if that handling ever regresses. Used by ``_get_env_config`` to detect env vars whose remainder claims a longer, sibling-owned prefix (e.g. ``SPECKIT_GIT_HOOKS_URL`` is diff --git a/src/specify_cli/integrations/catalog.py b/src/specify_cli/integrations/catalog.py index 1794caad83..b3be8a84e3 100644 --- a/src/specify_cli/integrations/catalog.py +++ b/src/specify_cli/integrations/catalog.py @@ -207,7 +207,7 @@ def _fetch_single_catalog( max_bytes=MAX_JSON_METADATA_BYTES, error_type=IntegrationCatalogError, label=f"catalog from {entry.url}", - ) + ).decode("utf-8") ) shape_error = _catalog_shape_error(catalog_data) diff --git a/src/specify_cli/integrations/hermes/__init__.py b/src/specify_cli/integrations/hermes/__init__.py index ec551902c7..072542923a 100644 --- a/src/specify_cli/integrations/hermes/__init__.py +++ b/src/specify_cli/integrations/hermes/__init__.py @@ -132,13 +132,27 @@ def setup( pfx = COMMAND_PREFIX if aliased != f"speckit.{command_name}" else "speckit" skill_name = f"{pfx}-{command_name.replace('.', '-')}" - # Parse frontmatter for description + # Parse frontmatter for description. Locate the closing ``---`` on + # its own line rather than with ``raw.split("---", 2)`` — a bare + # substring split stops at the first ``---`` *anywhere*, including + # one inside a value such as ``description: Separate sections + # with ---``, which truncates the frontmatter and drops later keys. + # The block between the delimiters is parsed unstripped so trailing + # newlines in literal (``|``) block scalars survive. frontmatter: dict[str, Any] = {} if raw.startswith("---"): - parts = raw.split("---", 2) - if len(parts) >= 3: + fm_lines = raw.splitlines(keepends=True) + fm_close = next( + ( + i + for i in range(1, len(fm_lines)) + if fm_lines[i].rstrip() == "---" + ), + None, + ) + if fm_close is not None: try: - fm = yaml.safe_load(parts[1]) + fm = yaml.safe_load("".join(fm_lines[1:fm_close])) if isinstance(fm, dict): frontmatter = fm except yaml.YAMLError: @@ -154,10 +168,26 @@ def setup( project_root=project_root, ) # Strip the processed frontmatter — we rebuild it for skills. + # Scan for the closing ``---`` on its own line rather than + # ``split("---", 2)`` so a ``---`` embedded in a value does not + # truncate the frontmatter and spill it into the body. if processed_body.startswith("---"): - parts = processed_body.split("---", 2) - if len(parts) >= 3: - processed_body = parts[2] + body_lines = processed_body.splitlines(keepends=True) + close_idx = next( + ( + i + for i in range(1, len(body_lines)) + if body_lines[i].rstrip() == "---" + ), + None, + ) + if close_idx is not None: + # Keep whatever trails the ``---`` marker on the closing + # line so the body stays byte-for-byte identical to + # ``split("---", 2)[2]`` for well-formed templates. + processed_body = body_lines[close_idx][3:] + "".join( + body_lines[close_idx + 1 :] + ) # Select description description = frontmatter.get("description", "") diff --git a/src/specify_cli/integrations/kimi/__init__.py b/src/specify_cli/integrations/kimi/__init__.py index 4517fac037..3a289d60ed 100644 --- a/src/specify_cli/integrations/kimi/__init__.py +++ b/src/specify_cli/integrations/kimi/__init__.py @@ -323,14 +323,24 @@ def _is_speckit_generated_skill(skill_dir: Path) -> bool: if not content.startswith("---"): return False - parts = content.split("---", 2) - if len(parts) < 3: + # Locate the closing ``---`` on its own line rather than with + # ``content.split("---", 2)`` — a bare substring split stops at the first + # ``---`` *anywhere*, including one inside a value such as + # ``description: Separate sections with ---``, which truncates the parsed + # frontmatter and can drop the metadata block this check relies on (so a + # Speckit-generated skill would not be recognized on teardown). + lines = content.splitlines(keepends=True) + close_idx = next( + (i for i in range(1, len(lines)) if lines[i].rstrip() == "---"), + None, + ) + if close_idx is None: return False try: import yaml - frontmatter = yaml.safe_load(parts[1]) + frontmatter = yaml.safe_load("".join(lines[1:close_idx])) except Exception: return False diff --git a/src/specify_cli/integrations/manifest.py b/src/specify_cli/integrations/manifest.py index ef2a9fc893..bde83f000f 100644 --- a/src/specify_cli/integrations/manifest.py +++ b/src/specify_cli/integrations/manifest.py @@ -451,8 +451,7 @@ def save(self) -> Path: _ensure_safe_manifest_destination(self.project_root, path) os.replace(temp_path, path) finally: - if temp_path.exists(): - temp_path.unlink() + temp_path.unlink(missing_ok=True) return path @classmethod diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index c2ff312c51..cbca0260db 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -5716,7 +5716,7 @@ def resolve_content( if not layers: return None - def _read_layer_content(layer: Dict[str, Any]) -> str: + def _read_layer_content(layer: Dict[str, Any]) -> Optional[str]: """Read a layer's raw text, rewriting extension-relative subdir references (agents/, knowledge-base/, etc.) to their installed location when the layer is extension-provided (#2101). @@ -5726,8 +5726,18 @@ def _read_layer_content(layer: Dict[str, Any]) -> str: rewrite when it wins outright above or serves as the composition base below — never as a mid-stack composing (append/prepend/wrap) layer. + + Returns None when the layer cannot be read or decoded: + collect_all_layers deliberately keeps a non-UTF-8 legacy layer + (with its "replace" default) so unrelated commands still + resolve, so the same tolerance must apply here — the documented + contract is "Composed content string, or None if not found", + not a raw UnicodeDecodeError at composition time. """ - text = layer["path"].read_text(encoding="utf-8") + try: + text = layer["path"].read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + return None extension_id = layer.get("extension_id") extension_dir = layer.get("extension_dir") if extension_id and extension_dir: @@ -5765,6 +5775,8 @@ def _read_layer_content(layer: Dict[str, Any]) -> str: # Convert to reversed_layers index base_reversed_idx = len(layers) - 1 - base_layer_idx content = _read_layer_content(layers[base_layer_idx]) + if content is None: + return None # Compose only the layers above the base (higher priority = lower index in layers, # higher index in reversed_layers). Process bottom-up from base+1. start_idx = base_reversed_idx + 1 @@ -5808,7 +5820,12 @@ def _split_frontmatter(text: str) -> tuple: # Apply composition layers from bottom to top for layer in reversed_layers[start_idx:]: - layer_content = layer["path"].read_text(encoding="utf-8") + try: + layer_content = layer["path"].read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + # Same tolerance as _read_layer_content: an unreadable layer + # means the composed result cannot be produced. + return None strategy = layer["strategy"] if is_command: diff --git a/src/specify_cli/shared_infra.py b/src/specify_cli/shared_infra.py index eab8accb7f..135b577191 100644 --- a/src/specify_cli/shared_infra.py +++ b/src/specify_cli/shared_infra.py @@ -30,6 +30,22 @@ def accent(text: str, bold: bool = False, italic: bool = False, dim: bool = Fals logger = logging.getLogger(__name__) +# Managed ``.specify/.gitignore``. Keeps machine-local Spec Kit state out of +# version control while leaving shareable project files (specs, constitution, +# templates, scripts, extension config) tracked. Patterns are relative to the +# ``.specify/`` directory the file lives in. +SPECIFY_GITIGNORE_CONTENT = """\ +# Machine-local Spec Kit state — not meant to be shared. +# Managed by the Specify CLI; safe to edit (your changes are preserved on refresh). + +# Local pointer to the current feature directory. Rewritten every time you +# switch features, so it is per-checkout state rather than something to share. +feature.json + +# Per-machine extension config overrides. +extensions/*/local-config.yml +""" + # Matches a SHA-256 digest in its normalized form: exactly 64 hexadecimal # characters. Callers lowercase the declared value before matching (see # ``expected_hex = raw.lower()`` below), so an uppercase digest is accepted and @@ -276,8 +292,7 @@ def _write_shared_bytes( _ensure_safe_shared_destination(project_path, dest) os.replace(temp_path, dest) finally: - if temp_path.exists(): - temp_path.unlink() + temp_path.unlink(missing_ok=True) _BASH_FORMAT_COMMAND_RE = re.compile( @@ -626,6 +641,36 @@ def _ensure_or_bucket_dir(directory: Path) -> bool: ) planned_templates.append((dst, rel, content)) + # Managed ``.specify/.gitignore`` — keeps machine-local state (the + # ``feature.json`` pointer and per-machine ``local-config.yml`` overrides) + # out of git while leaving everything else shareable. Routed through the + # same overwrite/skip/preserve policy as templates so ``--force`` refreshes + # it and user edits are preserved. Like every other shared-infra file it is + # tracked in ``speckit.manifest.json`` (not the per-integration manifest) and + # is therefore intentionally left in place by ``integration uninstall``. + specify_dir = project_path / ".specify" + if _ensure_or_bucket_dir(specify_dir): + gitignore_dst = specify_dir / ".gitignore" + gitignore_rel = gitignore_dst.relative_to(project_path).as_posix() + seen_rels.add(gitignore_rel) + if _safe_dest_or_bucket(gitignore_dst, gitignore_rel): + write, bucket = _decide_overwrite(gitignore_rel, gitignore_dst) + if write: + planned_templates.append( + (gitignore_dst, gitignore_rel, SPECIFY_GITIGNORE_CONTENT) + ) + elif bucket == "preserved": + preserved_user_files.append(gitignore_rel) + else: + skipped_files.append(gitignore_rel) + if gitignore_dst.is_file() and gitignore_rel not in prior_hashes: + try: + manifest.record_existing(gitignore_rel, recovered=True) + except (OSError, ValueError) as exc: + console.print( + f"[yellow]⚠[/yellow] could not record {gitignore_rel} in manifest: {exc}" + ) + for dst_path, rel, content, mode in planned_copies: if not _ensure_or_bucket_dir(dst_path.parent): continue diff --git a/src/specify_cli/workflows/_commands.py b/src/specify_cli/workflows/_commands.py index aee71e3fed..d4ea7acb45 100644 --- a/src/specify_cli/workflows/_commands.py +++ b/src/specify_cli/workflows/_commands.py @@ -1615,6 +1615,12 @@ def workflow_status( except ValueError as exc: err.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") raise typer.Exit(1) + except OSError as exc: + # An unreadable state.json (bad permissions, a directory in its + # place, I/O error) must fail as cleanly as the malformed-JSON + # case above -- `workflow resume` already handles OSError here. + err.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") + raise typer.Exit(1) if json_output: # Build on the shared run/resume payload so the common fields diff --git a/src/specify_cli/workflows/engine.py b/src/specify_cli/workflows/engine.py index a478aafddb..835183a2cb 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -1710,8 +1710,13 @@ def list_runs(self) -> list[dict[str, Any]]: continue state_path = run_dir / "state.json" if state_path.exists(): - with open(state_path, encoding="utf-8") as f: - state_data = json.load(f) + try: + with open(state_path, encoding="utf-8") as f: + state_data = json.load(f) + except (json.JSONDecodeError, OSError, UnicodeDecodeError): + continue + if not isinstance(state_data, dict) or "run_id" not in state_data: + continue runs.append(state_data) return runs diff --git a/tests/extensions/test_update_agent_context_python_parity.py b/tests/extensions/test_update_agent_context_python_parity.py index dc7ff3fcb9..350dc7edca 100644 --- a/tests/extensions/test_update_agent_context_python_parity.py +++ b/tests/extensions/test_update_agent_context_python_parity.py @@ -344,14 +344,19 @@ def test_python_mtime_fallback_matching_bash(tmp_path: Path) -> None: @requires_posix_bash -def test_python_mtime_fallback_finds_nested_plan_matching_bash(tmp_path: Path) -> None: - # Regression: the mtime fallback must discover plan.md in nested scoped - # layouts (specs///plan.md), matching the Bash/PowerShell - # ports and the documented recursive-discovery contract (see #3024). A - # one-level scan (specs/*/plan.md) would miss this and omit the plan link. +def test_python_mtime_fallback_finds_nested_plan_matching_bash( + tmp_path: Path, +) -> None: + """The mtime fallback must recurse into scoped layouts. + + A plan created under specs///plan.md (as produced via + SPECIFY_FEATURE_DIRECTORY) is more than one level below specs/. The old + Python port used a one-level specs/*/plan.md glob and missed it, while the + bash/PowerShell twins recurse (#3024). This locks in the parity. + """ repo_a, repo_b = twin_projects(tmp_path, context_file="AGENTS.md") for repo in (repo_a, repo_b): - plan = repo / "specs" / "scope-a" / "002-nested" / "plan.md" + plan = repo / "specs" / "backend" / "001-nested" / "plan.md" plan.parent.mkdir(parents=True, exist_ok=True) plan.write_text("# plan\n", encoding="utf-8") @@ -361,7 +366,39 @@ def test_python_mtime_fallback_finds_nested_plan_matching_bash(tmp_path: Path) - assert_parity(bash, py, repo_a, repo_b) content = (repo_b / "AGENTS.md").read_bytes() assert content == (repo_a / "AGENTS.md").read_bytes() - assert b"at specs/scope-a/002-nested/plan.md" in content + assert b"at specs/backend/001-nested/plan.md" in content + + +@requires_posix_bash +def test_python_mtime_fallback_skips_plan_reached_through_escaping_symlink( + tmp_path: Path, +) -> None: + """A plan reached via a specs/ symlink out of the project is not selected. + + ``relative_to()`` is lexical, so ``specs/linked/001-x/plan.md`` looks + in-project even when ``specs/linked`` points outside the tree. Resolving + before the containment check rejects it, so the fallback finds nothing and + the ``at `` line is omitted rather than naming an out-of-project file + with an in-project-looking path. Mirrors the bash twin's ``_resolved_rel``. + """ + repo_a, repo_b = twin_projects(tmp_path, context_file="AGENTS.md") + for repo in (repo_a, repo_b): + outside = repo.parent / f"outside-{repo.name}" / "001-x" + outside.mkdir(parents=True, exist_ok=True) + (outside / "plan.md").write_text("# plan\n", encoding="utf-8") + specs = repo / "specs" + specs.mkdir(parents=True, exist_ok=True) + (specs / "linked").symlink_to(outside.parent, target_is_directory=True) + # Sanity: the plan really is reachable through the symlink. + assert (specs / "linked" / "001-x" / "plan.md").is_file() + + bash = run_bash(repo_a) + py = run_python(repo_b) + + assert_parity(bash, py, repo_a, repo_b) + content = (repo_b / "AGENTS.md").read_bytes() + assert content == (repo_a / "AGENTS.md").read_bytes() + assert b"\nat " not in content @requires_posix_bash @@ -508,6 +545,31 @@ def test_python_fresh_context_file_matches_powershell(tmp_path: Path) -> None: assert (repo_a / "AGENTS.md").read_bytes() == (repo_b / "AGENTS.md").read_bytes() +@pytest.mark.skipif(not POWERSHELL, reason="no PowerShell available") +def test_python_mtime_fallback_finds_nested_plan_matches_powershell( + tmp_path: Path, +) -> None: + """Python's mtime fallback must recurse like the PowerShell twin. + + With no feature.json, discovery falls back to scanning under specs/. A plan + at specs///plan.md sits more than one level deep; the old + Python one-level glob missed it while PowerShell already recurses (#3024). + """ + repo_a = make_project(tmp_path / "proj-ps", context_file="AGENTS.md") + repo_b = make_project(tmp_path / "proj-py", context_file="AGENTS.md") + for repo in (repo_a, repo_b): + plan = repo / "specs" / "backend" / "001-nested" / "plan.md" + plan.parent.mkdir(parents=True, exist_ok=True) + plan.write_text("# plan\n", encoding="utf-8") + + ps = run_powershell(repo_a) + py = run_python(repo_b) + + assert ps.returncode == py.returncode == 0, ps.stderr + py.stderr + assert (repo_a / "AGENTS.md").read_bytes() == (repo_b / "AGENTS.md").read_bytes() + assert b"at specs/backend/001-nested/plan.md" in (repo_b / "AGENTS.md").read_bytes() + + @pytest.mark.skipif(not POWERSHELL, reason="no PowerShell available") def test_python_upsert_matches_powershell(tmp_path: Path) -> None: repo_a = make_project(tmp_path / "proj-ps", context_file="AGENTS.md") diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index c65c8406d4..e28eae831f 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -1080,6 +1080,99 @@ def test_init_here_without_force_preserves_shared_infra(self, tmp_path): assert "not updated" in result.output + def test_init_here_force_reapplies_installed_presets(self, tmp_path, monkeypatch): + """Regression for #3990: init --here --force must call _register_presets_for_agent + after setup() so preset-composed files are not silently reverted to core.""" + from unittest.mock import MagicMock, patch + + from typer.testing import CliRunner + + from specify_cli import app + + project = tmp_path / "force-preset-reapply" + project.mkdir() + + old_cwd = os.getcwd() + try: + os.chdir(project) + runner = CliRunner() + + # First init to create a valid project structure. + result = runner.invoke(app, [ + "init", "--here", "--force", + "--integration", "claude", + "--script", "sh", + "--ignore-agent-tools", + ], catch_exceptions=False) + assert result.exit_code == 0, result.output + + # Second init --here --force: verify _register_presets_for_agent is called. + # Patch at the source module since init.py does a lazy import of these functions. + mock_presets = MagicMock() + mock_extensions = MagicMock() + with ( + patch( + "specify_cli.integrations._helpers._register_presets_for_agent", + mock_presets, + ), + patch( + "specify_cli.integrations._helpers._register_extensions_for_agent", + mock_extensions, + ), + ): + result2 = runner.invoke(app, [ + "init", "--here", "--force", + "--integration", "claude", + "--script", "sh", + "--ignore-agent-tools", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + + assert result2.exit_code == 0, result2.output + assert mock_presets.called, ( + "_register_presets_for_agent was not called during init --here --force" + ) + assert mock_extensions.called, ( + "_register_extensions_for_agent was not called during init --here --force" + ) + + def test_init_here_without_force_does_not_reapply_presets(self, tmp_path): + """Without --force (fresh project), _register_presets_for_agent should NOT be called.""" + from unittest.mock import MagicMock, patch + + from typer.testing import CliRunner + + from specify_cli import app + + project = tmp_path / "no-force-preset" + project.mkdir() + + old_cwd = os.getcwd() + try: + os.chdir(project) + runner = CliRunner() + mock_presets = MagicMock() + with patch( + "specify_cli.integrations._helpers._register_presets_for_agent", + mock_presets, + ): + result = runner.invoke(app, [ + "init", "--here", + "--integration", "claude", + "--script", "sh", + "--ignore-agent-tools", + ], catch_exceptions=False) + finally: + os.chdir(old_cwd) + + assert result.exit_code == 0, result.output + # On a fresh project without --force the reapply guard should not fire. + assert not mock_presets.called, ( + "_register_presets_for_agent should not be called on a fresh init without --force" + ) + + class TestForceExistingDirectory: """Tests for --force merging into an existing named directory.""" diff --git a/tests/integrations/test_integration_base_markdown.py b/tests/integrations/test_integration_base_markdown.py index 5d8c45b19c..1c1e592caf 100644 --- a/tests/integrations/test_integration_base_markdown.py +++ b/tests/integrations/test_integration_base_markdown.py @@ -248,6 +248,7 @@ def _expected_files(self, script_variant: str, project=None) -> list[str]: files.append(".specify/init-options.json") files.append(f".specify/integrations/{self.KEY}.manifest.json") files.append(".specify/integrations/speckit.manifest.json") + files.append(".specify/.gitignore") if script_variant == "sh": for name in ["check-prerequisites.sh", "common.sh", "create-new-feature.sh", diff --git a/tests/integrations/test_integration_base_skills.py b/tests/integrations/test_integration_base_skills.py index 30f23951e4..1dd0b6bf08 100644 --- a/tests/integrations/test_integration_base_skills.py +++ b/tests/integrations/test_integration_base_skills.py @@ -165,6 +165,91 @@ def test_skill_uses_template_descriptions(self, tmp_path): assert isinstance(fm["description"], str) assert len(fm["description"]) > 0, f"{f} has empty description" + def test_skill_frontmatter_preserves_multiline_description( + self, tmp_path, monkeypatch + ): + """A multiline (block-scalar) description must round-trip exactly. + + The hand-built SKILL.md frontmatter used to only escape backslash and + quote, so a block-scalar description was emitted with raw newlines inside + a double-quoted scalar and reparsed with those newlines collapsed to + spaces. The description must survive byte-for-byte.""" + from pathlib import Path + + i = get_integration(self.KEY) + # Hermes writes to ~/.hermes/skills/ — isolate Path.home() to prevent + # overwriting a developer's real global skill directory. + if self.KEY == "hermes": + home = tmp_path / "home" + home.mkdir(exist_ok=True) + monkeypatch.setattr(Path, "home", lambda: home) + + template = tmp_path / "sample.md" + template.write_text( + "---\n" + "description: |\n" + " first line\n" + " second line\n" + "scripts:\n" + " sh: scripts/bash/x.sh\n" + "---\n" + "Body\n", + encoding="utf-8", + ) + monkeypatch.setattr(i, "list_command_templates", lambda: [template]) + + m = IntegrationManifest(self.KEY, tmp_path) + created = i.setup(tmp_path, m) + skill_files = [f for f in created if f.name == "SKILL.md"] + assert len(skill_files) == 1 + + content = skill_files[0].read_text(encoding="utf-8") + fm = yaml.safe_load(content.split("---", 2)[1]) + assert "\n" in fm["description"] + assert fm["description"] == "first line\nsecond line\n" + + def test_skill_frontmatter_preserves_control_characters( + self, tmp_path, monkeypatch + ): + """A description carrying a C0/DEL control char must round-trip exactly. + + A control character can reach ``description`` via a YAML escape in the + source template (``"a\\x08b"`` parses to a real U+0008). The old + hand-built frontmatter only escaped backslash and quote, so the raw + control char landed inside the emitted double-quoted scalar and made the + SKILL.md unparseable / lossy. ``yaml_quote`` must escape it so the + value survives byte-for-byte.""" + from pathlib import Path + + i = get_integration(self.KEY) + # Hermes writes to ~/.hermes/skills/ — isolate Path.home() to prevent + # overwriting a developer's real global skill directory. + if self.KEY == "hermes": + home = tmp_path / "home" + home.mkdir(exist_ok=True) + monkeypatch.setattr(Path, "home", lambda: home) + + template = tmp_path / "sample.md" + template.write_text( + "---\n" + 'description: "a\\x08b\\ttab"\n' + "scripts:\n" + " sh: scripts/bash/x.sh\n" + "---\n" + "Body\n", + encoding="utf-8", + ) + monkeypatch.setattr(i, "list_command_templates", lambda: [template]) + + m = IntegrationManifest(self.KEY, tmp_path) + created = i.setup(tmp_path, m) + skill_files = [f for f in created if f.name == "SKILL.md"] + assert len(skill_files) == 1 + + content = skill_files[0].read_text(encoding="utf-8") + fm = yaml.safe_load(content.split("---", 2)[1]) + assert fm["description"] == "a\x08b\ttab" + def test_templates_are_processed(self, tmp_path): """Skill body must have placeholders replaced, not raw templates.""" install_preset_to(tmp_path) @@ -445,6 +530,7 @@ def _expected_files(self, script_variant: str, project: Path | None = None) -> l ".specify/integration.json", f".specify/integrations/{self.KEY}.manifest.json", ".specify/integrations/speckit.manifest.json", + ".specify/.gitignore", ".specify/memory/.constitution-template.json", ".specify/memory/constitution.md", ] diff --git a/tests/integrations/test_integration_base_toml.py b/tests/integrations/test_integration_base_toml.py index e8742f8038..7f6dc4ddd1 100644 --- a/tests/integrations/test_integration_base_toml.py +++ b/tests/integrations/test_integration_base_toml.py @@ -494,6 +494,7 @@ def _expected_files(self, script_variant: str, project=None) -> list[str]: files.append(".specify/init-options.json") files.append(f".specify/integrations/{self.KEY}.manifest.json") files.append(".specify/integrations/speckit.manifest.json") # FIXME: should be spec.manifest.json + files.append(".specify/.gitignore") if script_variant == "sh": for name in [ diff --git a/tests/integrations/test_integration_base_yaml.py b/tests/integrations/test_integration_base_yaml.py index 72ece45258..f85ba82139 100644 --- a/tests/integrations/test_integration_base_yaml.py +++ b/tests/integrations/test_integration_base_yaml.py @@ -418,6 +418,7 @@ def _expected_files(self, script_variant: str, project=None) -> list[str]: files.append(".specify/init-options.json") files.append(f".specify/integrations/{self.KEY}.manifest.json") files.append(".specify/integrations/speckit.manifest.json") + files.append(".specify/.gitignore") if script_variant == "sh": for name in [ diff --git a/tests/integrations/test_integration_catalog.py b/tests/integrations/test_integration_catalog.py index a323db512c..b89ec26f4b 100644 --- a/tests/integrations/test_integration_catalog.py +++ b/tests/integrations/test_integration_catalog.py @@ -209,24 +209,44 @@ class TestCatalogFetch: """Tests that use a local HTTP server stub via monkeypatch.""" def _patch_urlopen(self, monkeypatch, catalog_data): - """Patch authentication.http.open_url to return *catalog_data*.""" - import io - from unittest.mock import MagicMock - import specify_cli.authentication.http as _auth_http + """Patch authentication.http.open_url to return *catalog_data*. - if isinstance(catalog_data, (dict, list)): - raw = json.dumps(catalog_data).encode("utf-8") - else: - raw = str(catalog_data).encode("utf-8") + ``_fetch_single_catalog`` calls ``open_url`` (which uses an opener, not + the module-level ``urllib.request.urlopen``), so the mock must target + ``open_url`` directly. Each call gets a fresh ``FakeResponse`` so the + bounded-read offset resets per fetch (the default + community catalogs + are both fetched).""" - mock_resp = MagicMock() - mock_resp.read.side_effect = io.BytesIO(raw).read - mock_resp.headers = {} - mock_resp.geturl.return_value = "https://example.com/catalog.json" - mock_resp.__enter__ = lambda s: s - mock_resp.__exit__ = MagicMock(return_value=False) + class FakeResponse: + def __init__(self, data, url=""): + self._data = json.dumps(data).encode() + self._url = url if isinstance(url, str) else url.full_url + self._offset = 0 + + def read(self, size=-1): + if size == -1: + chunk = self._data[self._offset:] + self._offset = len(self._data) + else: + chunk = self._data[self._offset:self._offset + size] + self._offset += len(chunk) + return chunk + + def geturl(self): + return self._url - monkeypatch.setattr(_auth_http, "open_url", lambda url, timeout=10: mock_resp) + def __enter__(self): + return self + + def __exit__(self, *a): + pass + + def fake_open_url(url, timeout=10, **kwargs): + return FakeResponse(catalog_data, url) + + import specify_cli.authentication.http as _auth_http + + monkeypatch.setattr(_auth_http, "open_url", fake_open_url) def test_fetch_and_search_all(self, tmp_path, monkeypatch): monkeypatch.setenv("HOME", str(tmp_path)) @@ -256,6 +276,110 @@ def test_fetch_and_search_all(self, tmp_path, monkeypatch): ids = [r["id"] for r in results] assert "acme-coder" in ids + def test_poisoned_cache_shape_is_dropped_and_refetched(self, tmp_path, monkeypatch): + """A fresh-but-mis-shaped cache (e.g. integrations as a list) must be + dropped and refetched, not returned — otherwise it later crashes on + .items(). The cache path must clear the same shape checks as a fresh + fetch.""" + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("USERPROFILE", str(tmp_path)) + monkeypatch.delenv("SPECKIT_INTEGRATION_CATALOG_URL", raising=False) + (tmp_path / ".specify").mkdir() + cat = IntegrationCatalog(tmp_path) + + catalog = { + "schema_version": "1.0", + "updated_at": "2026-01-01T00:00:00Z", + "integrations": { + "acme-coder": { + "id": "acme-coder", "name": "Acme Coder", "version": "2.0.0", + "description": "Community integration", "author": "acme-org", + "tags": ["cli"], + }, + }, + } + self._patch_urlopen(monkeypatch, catalog) + cat.search() # populate the cache legitimately + + # Poison the cached payload (integrations as a list), keeping the fresh + # metadata so the age check passes and the cache branch is taken. + cache_dir = tmp_path / ".specify" / "integrations" / ".cache" + data_files = [ + f for f in cache_dir.glob("catalog-*.json") + if not f.name.endswith("-metadata.json") + ] + assert data_files, "cache was not populated" + data_files[0].write_text( + json.dumps({"schema_version": "1.0", "integrations": []}), + encoding="utf-8", + ) + + # The poisoned cache is dropped and the (valid) source is refetched. + results = cat.search() + assert "acme-coder" in [r["id"] for r in results] + + def test_fetch_rejects_oversized_catalog_response( + self, tmp_path, monkeypatch + ): + """Regression: _fetch_single_catalog must use read_response_limited + with MAX_JSON_METADATA_BYTES, not unbounded resp.read().""" + from specify_cli.integrations.catalog import ( + IntegrationCatalog, + IntegrationCatalogError, + ) + import specify_cli.integrations.catalog as catalog_module + + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("USERPROFILE", str(tmp_path)) + monkeypatch.delenv("SPECKIT_INTEGRATION_CATALOG_URL", raising=False) + (tmp_path / ".specify").mkdir() + cat = IntegrationCatalog(tmp_path) + + # Set limit very small so any response is oversized + monkeypatch.setattr(catalog_module, "MAX_JSON_METADATA_BYTES", 32) + + class _OversizedResponse: + def __init__(self): + self._data = b"x" * 64 + self._offset = 0 + + def read(self, size=-1): + if size == -1: + chunk = self._data[self._offset:] + self._offset = len(self._data) + else: + chunk = self._data[self._offset:self._offset + size] + self._offset += len(chunk) + return chunk + + def geturl(self): + return "https://example.com/catalog.json" + + def __enter__(self): + return self + + def __exit__(self, *a): + pass + + import specify_cli.authentication.http as _auth_http + + def fake_open_url(url, timeout=10, **kwargs): + return _OversizedResponse() + + monkeypatch.setattr(_auth_http, "open_url", fake_open_url) + + from specify_cli.integrations.catalog import IntegrationCatalogEntry + + entry = IntegrationCatalogEntry( + url="https://example.com/catalog.json", + name="test", + priority=1, + install_allowed=True, + ) + + with pytest.raises(IntegrationCatalogError, match="exceeds maximum size"): + cat._fetch_single_catalog(entry, force_refresh=True) + def test_search_by_tag(self, tmp_path, monkeypatch): monkeypatch.setenv("HOME", str(tmp_path)) monkeypatch.setenv("USERPROFILE", str(tmp_path)) @@ -578,8 +702,16 @@ class FakeResponse: def __init__(self, data, url=""): self._data = json.dumps(data).encode() self._url = url if isinstance(url, str) else url.full_url - def read(self): - return self._data + self._offset = 0 + + def read(self, size=-1): + if size == -1: + chunk = self._data[self._offset:] + self._offset = len(self._data) + else: + chunk = self._data[self._offset:self._offset + size] + self._offset += len(chunk) + return chunk def geturl(self): return self._url diff --git a/tests/integrations/test_integration_cline.py b/tests/integrations/test_integration_cline.py index 86b2e4960f..1917c11608 100644 --- a/tests/integrations/test_integration_cline.py +++ b/tests/integrations/test_integration_cline.py @@ -202,6 +202,7 @@ def _expected_files(self, script_variant: str, project=None) -> list[str]: files.append(".specify/init-options.json") files.append(f".specify/integrations/{self.KEY}.manifest.json") files.append(".specify/integrations/speckit.manifest.json") + files.append(".specify/.gitignore") if script_variant == "sh": for name in [ diff --git a/tests/integrations/test_integration_copilot.py b/tests/integrations/test_integration_copilot.py index 9138b9ad08..a890824dea 100644 --- a/tests/integrations/test_integration_copilot.py +++ b/tests/integrations/test_integration_copilot.py @@ -279,6 +279,7 @@ def test_complete_file_inventory_sh(self, tmp_path): ".specify/init-options.json", ".specify/integrations/copilot.manifest.json", ".specify/integrations/speckit.manifest.json", + ".specify/.gitignore", ".specify/scripts/bash/check-prerequisites.sh", ".specify/scripts/bash/common.sh", ".specify/scripts/bash/create-new-feature.sh", @@ -353,6 +354,7 @@ def test_complete_file_inventory_ps(self, tmp_path): ".specify/init-options.json", ".specify/integrations/copilot.manifest.json", ".specify/integrations/speckit.manifest.json", + ".specify/.gitignore", ".specify/scripts/powershell/check-prerequisites.ps1", ".specify/scripts/powershell/common.ps1", ".specify/scripts/powershell/create-new-feature.ps1", @@ -904,6 +906,7 @@ def test_complete_file_inventory_skills_sh(self, tmp_path): ".specify/integration.json", ".specify/integrations/copilot.manifest.json", ".specify/integrations/speckit.manifest.json", + ".specify/.gitignore", # Scripts (sh) ".specify/scripts/bash/check-prerequisites.sh", ".specify/scripts/bash/common.sh", diff --git a/tests/integrations/test_integration_generic.py b/tests/integrations/test_integration_generic.py index 2bbb910246..ae54afe493 100644 --- a/tests/integrations/test_integration_generic.py +++ b/tests/integrations/test_integration_generic.py @@ -344,6 +344,7 @@ def test_complete_file_inventory_sh(self, tmp_path): ".specify/integration.json", ".specify/integrations/generic.manifest.json", ".specify/integrations/speckit.manifest.json", + ".specify/.gitignore", ".specify/memory/.constitution-template.json", ".specify/memory/constitution.md", ".specify/scripts/bash/check-prerequisites.sh", @@ -411,6 +412,7 @@ def test_complete_file_inventory_ps(self, tmp_path): ".specify/integration.json", ".specify/integrations/generic.manifest.json", ".specify/integrations/speckit.manifest.json", + ".specify/.gitignore", ".specify/memory/.constitution-template.json", ".specify/memory/constitution.md", ".specify/scripts/powershell/check-prerequisites.ps1", diff --git a/tests/integrations/test_skill_frontmatter_quoting.py b/tests/integrations/test_skill_frontmatter_quoting.py index b42ad88459..c7e7ebb0e8 100644 --- a/tests/integrations/test_skill_frontmatter_quoting.py +++ b/tests/integrations/test_skill_frontmatter_quoting.py @@ -178,3 +178,66 @@ def test_multiline_description_survives(self, tmp_path, monkeypatch): fm = _parse_frontmatter(skill_files[0]) assert fm["description"] == MULTILINE + + def test_dashed_description_is_preserved(self, tmp_path, monkeypatch): + """Hermes overrides setup(), so it needs the same line-anchored parse.""" + home = tmp_path / "home" + home.mkdir(exist_ok=True) + monkeypatch.setattr(Path, "home", lambda: home) + + integration = get_integration("hermes") + monkeypatch.setattr( + integration, + "shared_commands_dir", + lambda: _fake_templates(tmp_path, DASHED_TEMPLATE), + ) + manifest = IntegrationManifest("hermes", tmp_path) + created = integration.setup(tmp_path, manifest) + skill_files = [f for f in created if f.name == "SKILL.md"] + assert len(skill_files) == 1 + + fm = _parse_frontmatter_line_anchored(skill_files[0]) + assert fm["description"] == DASHED_DESCRIPTION + + content = skill_files[0].read_text(encoding="utf-8") + lines = content.splitlines(keepends=True) + end = next(i for i in range(1, len(lines)) if lines[i].rstrip() == "---") + body = "".join(lines[end + 1 :]) + assert "name-marker: sentinel" not in body + + +class TestKimiGeneratedSkillDetection: + """``_is_speckit_generated_skill`` must survive a ``---`` in a value. + + Teardown only removes a legacy skill directory it recognizes as + Speckit-generated via the frontmatter ``metadata`` block. A substring split + truncated the frontmatter before ``metadata`` when a description embedded + ``---``, so the directory was left behind on uninstall. + """ + + def _write_skill(self, skill_dir: Path, description: str) -> None: + skill_dir.mkdir(parents=True, exist_ok=True) + (skill_dir / "SKILL.md").write_text( + "---\n" + 'name: "speckit-plan"\n' + f"description: {description}\n" + "metadata:\n" + ' author: "github-spec-kit"\n' + ' source: "templates/commands/plan.md"\n' + "---\n\nBody.\n", + encoding="utf-8", + ) + + def test_detects_skill_with_dashes_in_description(self, tmp_path): + from specify_cli.integrations.kimi import _is_speckit_generated_skill + + skill_dir = tmp_path / "speckit-plan" + self._write_skill(skill_dir, "Separate sections with --- markers") + assert _is_speckit_generated_skill(skill_dir) is True + + def test_still_detects_plain_description(self, tmp_path): + from specify_cli.integrations.kimi import _is_speckit_generated_skill + + skill_dir = tmp_path / "speckit-plan" + self._write_skill(skill_dir, "Plain description") + assert _is_speckit_generated_skill(skill_dir) is True diff --git a/tests/test_check_prerequisites_python_parity.py b/tests/test_check_prerequisites_python_parity.py index cdc02b915d..5c5083f61f 100644 --- a/tests/test_check_prerequisites_python_parity.py +++ b/tests/test_check_prerequisites_python_parity.py @@ -181,6 +181,52 @@ def test_python_text_output_matches_bash(prereq_repo: Path) -> None: assert _normalize_status_text(py.stdout) == _normalize_status_text(bash.stdout) +def test_python_text_output_survives_a_legacy_stdout_code_page( + prereq_repo: Path, +) -> None: + """Text mode must not crash when stdout cannot encode the status glyphs. + + On Windows sys.stdout falls back to the ANSI code page whenever it is not a + console — which is every time an agent or a workflow step captures the + output. U+2713 is unencodable in cp1252, so printing it raised + UnicodeEncodeError and truncated the report right after "AVAILABLE_DOCS:". + The ASCII fallback is the rendering these markers already have in-tree + (Test-FileExists in scripts/powershell/common.ps1, and + normalize_status_text here). + """ + feat = prereq_repo / "specs" / "001-my-feature" + feat.mkdir(parents=True) + (feat / "plan.md").write_text("# plan\n", encoding="utf-8") + # research.md is present and the rest are not, so BOTH status markers are + # produced in the same cp1252 subprocess: U+2713 for the available document + # and U+2717 for the missing ones. Asserting only one of them would let a + # fallback that always returned "[FAIL]" pass. + (feat / "research.md").write_text("# research\n", encoding="utf-8") + (feat / "contracts").mkdir() # present but empty -> reported missing + _write_feature_json(prereq_repo) + + env = _clean_env() + env["PYTHONIOENCODING"] = "cp1252" + result = _run(_py_cmd(prereq_repo, "--include-tasks"), prereq_repo, env=env) + + assert result.returncode == 0, result.stderr + assert "UnicodeEncodeError" not in result.stderr + assert "AVAILABLE_DOCS:" in result.stdout + # Every per-document line must still be there, not truncated away by the + # encode error. + for doc in ( + "research.md", + "data-model.md", + "contracts/", + "quickstart.md", + "tasks.md", + ): + assert doc in result.stdout, (doc, result.stdout) + # Both fallback markers, so neither branch of _status_marker can regress. + assert "[OK] research.md" in result.stdout, result.stdout + assert "[FAIL] quickstart.md" in result.stdout, result.stdout + + @requires_bash def test_python_help_output_matches_bash(prereq_repo: Path) -> None: bash = _run(_bash_cmd(prereq_repo, "--help"), prereq_repo) diff --git a/tests/test_download_security.py b/tests/test_download_security.py index df6f9180d4..6f47b06cb5 100644 --- a/tests/test_download_security.py +++ b/tests/test_download_security.py @@ -475,6 +475,194 @@ def test_safe_extract_tar_enforces_entry_and_size_limits(tmp_path): safe_extract_tar(archive_path, tmp_path / "total", max_total_bytes=7) +def _truncated_tar_gz_bytes(keep_bytes): + """Return the leading *keep_bytes* of a multi-member tar.gz's bytes. + + A gzip stream cut short this way ends before its end-of-stream marker, so + reading it raises a bare ``EOFError`` from the gzip layer. ``tarfile`` + decompresses lazily, so *where* that surfaces depends on how much is kept: + a very short prefix fails in ``tarfile.open`` itself, while a longer one + opens fine and only fails once members are iterated. + """ + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w:gz") as archive: + for index in range(5): + info = tarfile.TarInfo(f"file{index}.txt") + content = bytes(range(256)) * 400 + info.size = len(content) + archive.addfile(info, io.BytesIO(content)) + return buffer.getvalue()[:keep_bytes] + + +def test_detect_archive_format_rejects_truncated_tar_gz(tmp_path): + # A gzip stream truncated before tarfile can read its first header raises a + # bare EOFError -- not a TarError -- from the format probe. Catching only + # TarError let it escape as a raw exception instead of leaving is_tar_gz + # False and reporting the module's clean format-mismatch error. + archive_path = tmp_path / "truncated.tar.gz" + archive_path.write_bytes(_truncated_tar_gz_bytes(64)) + + with pytest.raises(ValueError, match="format mismatch"): + detect_archive_format(archive_path) + + +@pytest.mark.parametrize("keep_bytes", [64, 512, 2048]) +def test_safe_extract_tar_rejects_truncated_archive(tmp_path, keep_bytes): + # The same bare EOFError, from tarfile.open on a short prefix and from + # member iteration on a longer one. Both sites reported it raw. + archive_path = tmp_path / f"truncated-{keep_bytes}.tar.gz" + archive_path.write_bytes(_truncated_tar_gz_bytes(keep_bytes)) + + with pytest.raises(ValueError, match="Invalid tar.gz archive"): + safe_extract_tar(archive_path, tmp_path / f"out-{keep_bytes}") + + +def test_safe_extract_tar_wraps_truncation_in_caller_error_type(tmp_path): + # The leak bypassed the caller's domain error type entirely, so callers + # that only catch their own error (or ValueError) crashed the command. + archive_path = tmp_path / "truncated.tar.gz" + archive_path.write_bytes(_truncated_tar_gz_bytes(2048)) + + with pytest.raises(_CustomZipError, match="Invalid tar.gz archive"): + safe_extract_tar( + archive_path, + tmp_path / "out", + error_type=_CustomZipError, + ) + + +def test_safe_extract_archive_rejects_truncated_tar_gz(tmp_path): + archive_path = tmp_path / "truncated.tar.gz" + archive_path.write_bytes(_truncated_tar_gz_bytes(2048)) + + with pytest.raises(ValueError): + safe_extract_archive(archive_path, tmp_path / "out") + + +#: Bytes of the first member's data that decompress cleanly before the invalid +#: deflate block. Must exceed the gzip read buffer so ``tarfile`` has to seek +#: forward over member data to reach the second header -- see +#: ``_corrupt_deflate_tar_gz_bytes``. The members are twice this size, so the +#: corruption stays well inside the first member's data. +_CORRUPT_DEFLATE_CLEAN_BYTES = 256 * 1024 +_CORRUPT_DEFLATE_MEMBER_BYTES = 2 * _CORRUPT_DEFLATE_CLEAN_BYTES + + +def _corrupt_deflate_tar_gz_bytes(): + """Return a tar.gz whose deflate stream is corrupt mid-member. + + Unlike truncation, which the gzip layer reports as ``EOFError``, an invalid + deflate block raises ``zlib.error``. ``tarfile`` converts that to + ``ReadError`` when it surfaces while reading a member *header*, but the + forward seek it performs to skip over member *data* sits outside that + conversion, so the raw ``zlib.error`` escapes from there. + + Two details keep this deterministic across zlib versions: + + * The corruption is a block header whose ``BTYPE`` is the reserved value + ``0b11``, which every zlib rejects as "invalid block type". Mangling + arbitrary bytes instead is *not* portable -- the garbage may still decode + structurally and fail the later gzip CRC check as ``BadGzipFile`` (an + ``OSError``, which the handler already caught) rather than raising + ``zlib.error`` at all. + * The stream is assembled by hand so the invalid block lands after + ``_CORRUPT_DEFLATE_CLEAN_BYTES`` of valid data. That is past the gzip read + buffer, so the first header reads clean and the failure happens during the + seek over member data rather than during a header read. + """ + plain = io.BytesIO() + with tarfile.open(fileobj=plain, mode="w") as archive: + for index in range(2): + info = tarfile.TarInfo(f"file{index}.txt") + content = bytes((i * 7 + index) % 256 for i in range(1024)) * ( + _CORRUPT_DEFLATE_MEMBER_BYTES // 1024 + ) + info.size = len(content) + archive.addfile(info, io.BytesIO(content)) + + clean_prefix = plain.getvalue()[:_CORRUPT_DEFLATE_CLEAN_BYTES] + compressor = zlib.compressobj(1, zlib.DEFLATED, -15) + deflate = compressor.compress(clean_prefix) + deflate += compressor.flush(zlib.Z_SYNC_FLUSH) + deflate += b"\x06" # BTYPE=0b11 (reserved) -> "invalid block type" + + gzip_header = b"\x1f\x8b\x08\x00" + b"\x00" * 4 + b"\x00\xff" + trailer = struct.pack(" ReadError conversion, so the probe sees ReadError. The + # zlib.error arm of _TAR_DECOMPRESSION_ERRORS is defensive at this site and + # load-bearing only at the two safe_extract_tar sites. + archive_path = tmp_path / "corrupt.tar.gz" + archive_path.write_bytes(_corrupt_deflate_tar_gz_bytes()) + + assert detect_archive_format(archive_path) == "tar.gz" + + +def test_safe_extract_tar_rejects_corrupt_deflate(tmp_path): + archive_path = tmp_path / "corrupt.tar.gz" + archive_path.write_bytes(_corrupt_deflate_tar_gz_bytes()) + + with pytest.raises(ValueError, match="Invalid tar.gz archive"): + safe_extract_tar(archive_path, tmp_path / "out") + + +def test_safe_extract_tar_wraps_corrupt_deflate_in_caller_error_type(tmp_path): + # zlib.error must reach the caller's domain error type, exactly as EOFError + # does, so this cannot regress independently of the truncation handling. + archive_path = tmp_path / "corrupt.tar.gz" + archive_path.write_bytes(_corrupt_deflate_tar_gz_bytes()) + + with pytest.raises(_CustomZipError, match="Invalid tar.gz archive"): + safe_extract_tar( + archive_path, + tmp_path / "out", + error_type=_CustomZipError, + ) + + +def test_safe_extract_archive_wraps_corrupt_deflate_in_caller_error_type(tmp_path): + archive_path = tmp_path / "corrupt.tar.gz" + archive_path.write_bytes(_corrupt_deflate_tar_gz_bytes()) + + with pytest.raises(_CustomZipError, match="Invalid tar.gz archive"): + safe_extract_archive( + archive_path, + tmp_path / "out", + error_type=_CustomZipError, + ) + + @pytest.mark.parametrize("suffix", [".zip", ".tar.gz", ".tgz"]) def test_safe_extract_archive_has_format_parity(tmp_path, suffix): archive_path = tmp_path / f"package{suffix}" diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 9d0c3d0f3d..5698bf357e 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -17,6 +17,7 @@ import tempfile import shutil import tomllib +import yaml from contextlib import contextmanager from pathlib import Path from datetime import datetime, timezone @@ -1290,6 +1291,31 @@ def test_list_returns_empty_dict_for_corrupted_registry(self, temp_dir): result = registry.list() assert result == {} + def test_load_starts_fresh_for_non_utf8_registry(self, temp_dir): + """A registry file with undecodable bytes must start fresh, not raise. + + ``_load()`` already treats malformed JSON as "corrupted registry, + start fresh", but a registry whose *bytes* cannot be decoded as UTF-8 + raised a raw ``UnicodeDecodeError`` from the text-mode read before + JSON parsing began — the same corruption class reaching a different + exception type. Because the registry is loaded in ``__init__``, that + traceback broke *every* extension command on the project. + """ + extensions_dir = temp_dir / "extensions" + extensions_dir.mkdir() + (extensions_dir / ExtensionRegistry.REGISTRY_FILE).write_bytes( + b"\xff\xfe not utf-8 \xc3\x28" + ) + + registry = ExtensionRegistry(extensions_dir) + + assert registry.data == { + "schema_version": ExtensionRegistry.SCHEMA_VERSION, + "extensions": {}, + } + assert registry.list() == {} + assert not registry.is_installed("test-ext") + # ===== ExtensionManager Tests ===== @@ -2996,6 +3022,30 @@ def test_render_frontmatter_unicode(self): assert "Prüfe Konformität" in output assert "\\u" not in output + def test_render_frontmatter_keeps_long_description_on_one_line(self): + """A long description must not be folded across lines. + + PyYAML wraps plain scalars at ~80 columns by default, which splits a + long ``description`` onto a continuation line. The YAML stays valid, + but the rendered frontmatter then differs in shape from the + hand-written core command templates, where ``description`` is always a + single line -- and consumers that read frontmatter line-wise see a + truncated description followed by a stray line. + """ + long_description = ( + "Execute the implementation plan by processing and executing all " + "tasks defined in tasks.md" + ) + frontmatter = {"name": "speckit-implement", "description": long_description} + + registrar = CommandRegistrar() + output = registrar.render_frontmatter(frontmatter) + + assert f"description: {long_description}\n" in output + + body = output.split("---\n")[1] + assert yaml.safe_load(body)["description"] == long_description + def test_adjust_script_paths_does_not_mutate_input(self): """Path adjustments should not mutate caller-owned frontmatter dicts.""" from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar diff --git a/tests/test_init_output_markup.py b/tests/test_init_output_markup.py new file mode 100644 index 0000000000..54576fb33f --- /dev/null +++ b/tests/test_init_output_markup.py @@ -0,0 +1,176 @@ +"""`specify init` must render user-supplied values literally, not as Rich markup. + +`commands/init.py` interpolated the project name, `--integration`/`--script` +values and paths straight into Rich markup f-strings. A name containing a +tag-shaped bracket run was therefore consumed as markup: + +* ``specify init "proj [v2]"`` succeeded and created the directory, but the + Next Steps panel printed ``cd proj`` -- a command that fails when pasted. +* ``specify init "app[/red]x"`` created the directory and then died with + ``MarkupError``, so the user saw a traceback for a project that had in fact + been scaffolded. + +Every sibling CLI module (extensions, presets, workflows, integrations) already +escapes user-controlled display values; init.py was the outlier. +""" + +from __future__ import annotations + +import os +import re +import subprocess +from pathlib import Path + +import pytest +from typer.testing import CliRunner + +from specify_cli import app +from specify_cli.commands.init import _shell_quote_arg + +from tests.conftest import requires_bash + +_ANSI = re.compile(r"\x1b\[[0-9;]*m") + + +def _strip(text: str) -> str: + return _ANSI.sub("", text or "") + + +def _init(tmp_path: Path, name: str): + """Run a fully offline, non-interactive `specify init `.""" + previous = os.getcwd() + os.chdir(tmp_path) + try: + return CliRunner().invoke( + app, + [ + "init", + name, + "--integration", + "generic", + "--integration-options", + "--commands-dir .agent/commands", + "--ignore-agent-tools", + "--offline", + ], + catch_exceptions=True, + ) + finally: + os.chdir(previous) + + +@pytest.mark.parametrize("name", ["proj [v2]", "my[bold]app"]) +def test_next_steps_cd_shows_the_real_project_name(tmp_path: Path, name: str): + """The `cd` line must name the directory that was actually created.""" + result = _init(tmp_path, name) + assert result.exit_code == 0, _strip(result.stdout) + assert (tmp_path / name).is_dir() + + out = _strip(result.stdout) + cd_lines = [line for line in out.splitlines() if "cd " in line] + assert cd_lines, out + assert f"cd {_shell_quote_arg(name)}" in " ".join(cd_lines), cd_lines + + +def test_closing_tag_in_project_name_does_not_crash(tmp_path: Path): + """A name forming a closing tag raised MarkupError *after* the project had + been created, so init reported failure for work it had completed.""" + name = "app[/red]x" + result = _init(tmp_path, name) + + assert result.exception is None or not isinstance( + result.exception, Exception + ) or "MarkupError" not in type(result.exception).__name__, ( + f"unexpected {type(result.exception).__name__}: {result.exception}" + ) + assert result.exit_code == 0, _strip(result.stdout) + assert (tmp_path / name).is_dir() + assert f"cd {_shell_quote_arg(name)}" in _strip(result.stdout) + + +def test_invalid_integration_value_is_rendered_literally(tmp_path: Path): + """An invalid `--integration` value is echoed back; it must not be parsed as + markup (nor raise) when it contains a bracket run.""" + previous = os.getcwd() + os.chdir(tmp_path) + try: + result = CliRunner().invoke( + app, + ["init", "proj", "--integration", "nope[/red]", "--ignore-agent-tools"], + catch_exceptions=True, + ) + finally: + os.chdir(previous) + + assert result.exit_code != 0 + assert "nope[/red]" in _strip(result.stdout) + + +def _cd_argument(stdout: str) -> str: + """Return the argument of the printed `cd` command, verbatim. + + The line is rendered inside a Rich panel, so the trailing box-drawing + border and its padding are stripped before the argument is compared. + """ + marker = "Go to the project folder: cd " + for line in _strip(stdout).splitlines(): + if marker in line: + return line.split(marker, 1)[1].rstrip().rstrip("│").rstrip() + raise AssertionError(f"no cd line in output:\n{stdout}") + + +@pytest.mark.parametrize("name", ["proj v2", "my project"]) +def test_cd_line_quotes_a_name_containing_whitespace(tmp_path: Path, name: str): + """Rich-escaping alone left `cd proj v2`, which every shell reads as two + arguments, so the copy-pasted command did not enter the directory.""" + result = _init(tmp_path, name) + assert result.exit_code == 0, _strip(result.stdout) + assert (tmp_path / name).is_dir() + + printed = _cd_argument(result.stdout) + assert printed != name, "a whitespace-bearing name must be quoted" + assert name in printed, printed + assert printed == _shell_quote_arg(name) + + +def test_ordinary_name_is_not_quoted(tmp_path: Path): + """The common case must stay byte-identical: no gratuitous quoting.""" + result = _init(tmp_path, "my-project") + assert result.exit_code == 0, _strip(result.stdout) + assert _cd_argument(result.stdout) == "my-project" + + +@requires_bash +@pytest.mark.parametrize("name", ["proj v2", "proj [v2]", "my-project"]) +def test_printed_cd_command_actually_changes_directory(tmp_path: Path, name: str): + """Execute the printed command rather than only inspecting it. + + This is the assertion the string comparisons cannot make: the rendered + `cd ` is fed to a real shell and must land in the created directory. + """ + result = _init(tmp_path, name) + assert result.exit_code == 0, _strip(result.stdout) + target = tmp_path / name + assert target.is_dir() + + printed = _cd_argument(result.stdout) + proc = subprocess.run( + ["bash", "-c", f"cd {printed} && pwd"], + cwd=tmp_path, + capture_output=True, + text=True, + ) + assert proc.returncode == 0, f"cd {printed!r} failed: {proc.stderr}" + assert Path(proc.stdout.strip()).name == name, proc.stdout + + +def test_shell_quote_arg_is_host_appropriate(): + """The helper follows `_version._render_argv`: list2cmdline on Windows, + shlex.quote elsewhere. Names needing no quoting round-trip unchanged.""" + assert _shell_quote_arg("my-project") == "my-project" + quoted = _shell_quote_arg("my project") + assert quoted != "my project" + if os.name == "nt": + assert quoted == '"my project"' + else: + assert quoted == "'my project'" diff --git a/tests/test_presets.py b/tests/test_presets.py index adc8beaf3c..358583e4bd 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -11460,6 +11460,113 @@ def test_resolve_content_nonexistent(self, project_dir): content = resolver.resolve_content("nonexistent") assert content is None + def test_resolve_content_unreadable_winning_layer_returns_none(self, project_dir): + """An undecodable winning layer must yield None, not a raw traceback. + + ``collect_all_layers`` deliberately keeps a non-UTF-8 legacy command + layer (with its ``replace`` default) so unrelated commands still + resolve. ``resolve_content`` then read that same file without a + boundary, so the tolerated layer crashed with ``UnicodeDecodeError`` + at composition time — reachable from ``specify preset add`` via + ``_register_commands``. The documented contract is "Composed content + string, or None if not found". + """ + presets_dir = project_dir / ".specify" / "presets" + command_path = ( + presets_dir / "legacy-pack" / "commands" / "speckit.legacy.md" + ) + command_path.parent.mkdir(parents=True) + command_path.write_bytes(b"\xff\xfe") + PresetRegistry(presets_dir).add( + "legacy-pack", {"version": "1.0.0", "priority": 10} + ) + + resolver = PresetResolver(project_dir) + content = resolver.resolve_content("speckit.legacy", "command") + assert content is None + + def test_resolve_content_unreadable_base_under_composing_layer( + self, project_dir, temp_dir, valid_pack_data + ): + """An undecodable base beneath a valid composing layer yields None. + + Covers the base-read guard: the winning layer composes (append), so + resolution reads the base layer beneath it — here the core template, + corrupted to non-UTF-8 — and must return None instead of crashing. + """ + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": "append-pack", "name": "Append"} + pack_data["provides"] = { + "templates": [{ + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "strategy": "append", + }] + } + pack_dir = temp_dir / "append-pack" + pack_dir.mkdir() + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + (pack_dir / "templates").mkdir() + (pack_dir / "templates" / "spec-template.md").write_text("## Appended Section\n") + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") + + core_spec = project_dir / ".specify" / "templates" / "spec-template.md" + core_spec.write_bytes(b"\xff\xfe") + + resolver = PresetResolver(project_dir) + assert resolver.resolve_content("spec-template") is None + + def test_resolve_content_unreadable_composing_layer( + self, project_dir, temp_dir, valid_pack_data, monkeypatch + ): + """An unreadable composing layer over a valid base yields None. + + Covers the composition-loop read and the ``OSError`` half of the + boundary: the base (core template) reads fine, but the append layer + raises a mocked ``PermissionError`` — mocked so the case also holds + under privileged CI where permission bits are not enforced. + """ + pack_data = {**valid_pack_data} + pack_data["preset"] = {**valid_pack_data["preset"], "id": "append-pack", "name": "Append"} + pack_data["provides"] = { + "templates": [{ + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + "strategy": "append", + }] + } + pack_dir = temp_dir / "append-pack" + pack_dir.mkdir() + with open(pack_dir / "preset.yml", 'w') as f: + yaml.dump(pack_data, f) + (pack_dir / "templates").mkdir() + (pack_dir / "templates" / "spec-template.md").write_text("## Appended Section\n") + + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") + + layer_path = ( + project_dir / ".specify" / "presets" / "append-pack" + / "templates" / "spec-template.md" + ) + assert layer_path.is_file() + original_read_text = Path.read_text + + def failing_read_text(self_path, *args, **kwargs): + if self_path == layer_path: + raise PermissionError(13, "Permission denied") + return original_read_text(self_path, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", failing_read_text) + + resolver = PresetResolver(project_dir) + assert resolver.resolve_content("spec-template") is None + def test_resolve_content_replace_strategy(self, project_dir, temp_dir, valid_pack_data): """Test resolve_content with default replace strategy.""" manager = PresetManager(project_dir) diff --git a/tests/test_setup_tasks_python_parity.py b/tests/test_setup_tasks_python_parity.py index 29d0e2b5aa..afa303b6bd 100644 --- a/tests/test_setup_tasks_python_parity.py +++ b/tests/test_setup_tasks_python_parity.py @@ -205,3 +205,28 @@ def test_missing_template_error_matches_all_variants(repo: Path) -> None: assert bash.returncode == ps.returncode == py.returncode == 1 assert bash.stdout == ps.stdout == py.stdout == "" assert bash.stderr == ps.stderr == py.stderr + + +def test_python_text_output_survives_a_legacy_stdout_code_page(repo: Path) -> None: + """Text mode must not crash when stdout cannot encode the status glyphs. + + On Windows sys.stdout falls back to the ANSI code page whenever it is not a + console — which is every time an agent or a workflow step captures the + output. U+2713 is unencodable in cp1252, so printing it raised + UnicodeEncodeError and truncated the document listing. The ASCII fallback is + the rendering these markers already have in-tree (Test-FileExists in + scripts/powershell/common.ps1, and normalize_status_text). + """ + feature = repo / "specs" / "001-my-feature" + (feature / "research.md").write_text("# research\n", encoding="utf-8") + (feature / "contracts").mkdir() + + env = clean_env() + env["PYTHONIOENCODING"] = "cp1252" + result = run(py_cmd(repo, SCRIPT), repo, env=env) + + assert result.returncode == 0, result.stderr + assert "UnicodeEncodeError" not in result.stderr + for doc in ("research.md", "data-model.md", "contracts/", "quickstart.md"): + assert doc in result.stdout, (doc, result.stdout) + assert "[OK] research.md" in normalize_status_text(result.stdout), result.stdout diff --git a/tests/test_shared_infra_gitignore.py b/tests/test_shared_infra_gitignore.py new file mode 100644 index 0000000000..4badeaa8e4 --- /dev/null +++ b/tests/test_shared_infra_gitignore.py @@ -0,0 +1,103 @@ +"""Tests for the managed ``.specify/.gitignore`` written by shared-infra install. + +The Specify CLI scaffolds a ``.specify/.gitignore`` so machine-local Spec Kit +state (the ``feature.json`` current-feature pointer and per-machine extension +``local-config.yml`` overrides) stays out of version control while everything +else under ``.specify/`` remains shareable. These tests pin that behaviour: +the file is created and manifest-tracked, its patterns actually make git ignore +the intended paths, user edits are preserved on a plain re-run, and ``--force`` +restores the managed content. +""" + +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +from specify_cli import _install_shared_infra +from specify_cli.shared_infra import SPECIFY_GITIGNORE_CONTENT + + +def _install(project: Path, **kwargs) -> None: + (project / ".specify").mkdir(parents=True, exist_ok=True) + _install_shared_infra(project, "sh", **kwargs) + + +def test_gitignore_is_written_and_tracked(tmp_path: Path) -> None: + project = tmp_path / "proj" + _install(project) + + gitignore = project / ".specify" / ".gitignore" + assert gitignore.is_file() + + content = gitignore.read_text(encoding="utf-8") + assert "feature.json" in content + assert "extensions/*/local-config.yml" in content + + manifest = json.loads( + (project / ".specify" / "integrations" / "speckit.manifest.json").read_text( + encoding="utf-8" + ) + ) + assert ".specify/.gitignore" in manifest.get("files", {}) + + +@pytest.mark.skipif(shutil.which("git") is None, reason="git not available") +def test_git_ignores_the_intended_paths(tmp_path: Path) -> None: + project = tmp_path / "proj" + project.mkdir() + subprocess.run(["git", "init", "-q"], cwd=project, check=True) + + _install(project) + + (project / ".specify" / "feature.json").write_text("{}", encoding="utf-8") + ext_local = project / ".specify" / "extensions" / "git" / "local-config.yml" + ext_local.parent.mkdir(parents=True, exist_ok=True) + ext_local.write_text("x\n", encoding="utf-8") + + for rel in ( + ".specify/feature.json", + ".specify/extensions/git/local-config.yml", + ): + result = subprocess.run( + ["git", "check-ignore", rel], + cwd=project, + capture_output=True, + text=True, + ) + assert result.returncode == 0, f"{rel} was not ignored" + + # A shareable file under .specify/ must NOT be ignored. + tracked = subprocess.run( + ["git", "check-ignore", ".specify/memory/constitution.md"], + cwd=project, + capture_output=True, + text=True, + ) + assert tracked.returncode == 1 + + +def test_user_edits_preserved_by_default(tmp_path: Path) -> None: + project = tmp_path / "proj" + _install(project) + + gitignore = project / ".specify" / ".gitignore" + gitignore.write_text("# my customization\n", encoding="utf-8") + + _install(project) # plain re-run must not clobber user edits + assert gitignore.read_text(encoding="utf-8") == "# my customization\n" + + +def test_force_restores_managed_content(tmp_path: Path) -> None: + project = tmp_path / "proj" + _install(project) + + gitignore = project / ".specify" / ".gitignore" + gitignore.write_text("# my customization\n", encoding="utf-8") + + _install(project, force=True) + assert gitignore.read_text(encoding="utf-8") == SPECIFY_GITIGNORE_CONTENT diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 1f33f825a8..238e41e216 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -7410,6 +7410,94 @@ def test_list_after_execution(self, project_dir): assert len(runs) == 1 assert runs[0]["workflow_id"] == "list-test" + def test_list_skips_malformed_json(self, project_dir): + from specify_cli.workflows.engine import WorkflowEngine + + runs_dir = project_dir / ".specify" / "workflows" / "runs" + bad_dir = runs_dir / "bad-run" + bad_dir.mkdir(parents=True) + (bad_dir / "state.json").write_text("{invalid json", encoding="utf-8") + + engine = WorkflowEngine(project_dir) + assert engine.list_runs() == [] + + def test_list_skips_unreadable_file(self, project_dir): + import sys + import subprocess + from specify_cli.workflows.engine import WorkflowEngine + + runs_dir = project_dir / ".specify" / "workflows" / "runs" + bad_dir = runs_dir / "bad-run" + bad_dir.mkdir(parents=True) + state_file = bad_dir / "state.json" + state_file.write_text('{"run_id": "x"}', encoding="utf-8") + + if sys.platform == "win32": + subprocess.run(["attrib", "+R", str(state_file)], check=True) + else: + state_file.chmod(0o000) + + try: + engine = WorkflowEngine(project_dir) + if sys.platform == "win32": + assert engine.list_runs() == [{"run_id": "x"}] + else: + assert engine.list_runs() == [] + finally: + if sys.platform == "win32": + subprocess.run(["attrib", "-R", str(state_file)], check=True) + else: + state_file.chmod(0o644) + + def test_list_skips_non_dict_payload(self, project_dir): + from specify_cli.workflows.engine import WorkflowEngine + + runs_dir = project_dir / ".specify" / "workflows" / "runs" + bad_dir = runs_dir / "bad-run" + bad_dir.mkdir(parents=True) + (bad_dir / "state.json").write_text('["not", "a", "dict"]', encoding="utf-8") + + engine = WorkflowEngine(project_dir) + assert engine.list_runs() == [] + + def test_list_skips_empty_dict_payload(self, project_dir): + from specify_cli.workflows.engine import WorkflowEngine + + runs_dir = project_dir / ".specify" / "workflows" / "runs" + bad_dir = runs_dir / "bad-run" + bad_dir.mkdir(parents=True) + (bad_dir / "state.json").write_text('{}', encoding="utf-8") + + engine = WorkflowEngine(project_dir) + assert engine.list_runs() == [] + + def test_list_skips_bad_file_with_valid_sibling(self, project_dir): + from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition + + runs_dir = project_dir / ".specify" / "workflows" / "runs" + bad_dir = runs_dir / "bad-run" + bad_dir.mkdir(parents=True) + (bad_dir / "state.json").write_text("{bad", encoding="utf-8") + + yaml_str = """ +schema_version: "1.0" +workflow: + id: "good-run" + name: "Good Run" + version: "1.0.0" +steps: + - id: step-one + type: shell + run: "echo test" +""" + definition = WorkflowDefinition.from_string(yaml_str) + engine = WorkflowEngine(project_dir) + engine.execute(definition) + + runs = engine.list_runs() + assert len(runs) == 1 + assert runs[0]["workflow_id"] == "good-run" + # ===== Workflow Registry Tests ===== @@ -16695,6 +16783,57 @@ def _raise_value_error(*args, **kwargs): assert "corrupt run state" not in captured.out assert captured.out.strip() == "" + def test_status_unreadable_run_state_exits_cleanly( + self, project_dir, monkeypatch + ): + """`workflow status ` gained a ValueError boundary to match + `workflow resume`, but not resume's OSError one -- so an unreadable + state.json (bad permissions, a directory in its place, an I/O error) + still leaked a raw traceback. exists() is True for a directory, so + the guard passes and open() raises OSError.""" + from typer.testing import CliRunner + from specify_cli import app + + monkeypatch.chdir(project_dir) + runs_dir = project_dir / ".specify" / "workflows" / "runs" / "abc123" + runs_dir.mkdir(parents=True, exist_ok=True) + # A directory where state.json should be: exists() passes, open() fails. + (runs_dir / "state.json").mkdir(exist_ok=True) + + runner = CliRunner() + result = runner.invoke(app, ["workflow", "status", "abc123"]) + assert result.exit_code != 0 + assert result.exception is None or isinstance(result.exception, SystemExit) + assert "Error" in result.output + + def test_status_json_unreadable_run_state_error_goes_to_stderr( + self, project_dir, monkeypatch, capsys + ): + """The OSError handler must route to stderr under --json too, so the + stdout JSON stream stays parseable -- mirroring the sibling + FileNotFoundError/ValueError handlers.""" + import typer + from specify_cli.workflows import _commands + from specify_cli.workflows.engine import RunState + + (project_dir / ".specify" / "workflows").mkdir(parents=True, exist_ok=True) + monkeypatch.setattr( + _commands, "_require_specify_project", lambda: project_dir + ) + + def _raise_os_error(*args, **kwargs): + raise PermissionError(13, "Permission denied") + + monkeypatch.setattr(RunState, "load", _raise_os_error) + + with pytest.raises(typer.Exit) as exc: + _commands.workflow_status("some-run", json_output=True) + assert exc.value.exit_code == 1 + captured = capsys.readouterr() + assert "Permission denied" in captured.err + assert "Permission denied" not in captured.out + assert captured.out.strip() == "" + def test_status_no_run_id_list_path_unaffected(self, project_dir, monkeypatch): """The no-run-id list-all-runs path must remain unaffected by the new single-run ValueError boundary."""