From 1e85d4ff53d1d51adbb06b27fe602fd2a37d54dc Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Wed, 5 Aug 2026 18:11:38 +0500 Subject: [PATCH 01/20] fix: skip corrupted run state files in list_runs (#3814) * fix: skip corrupted run state files in list_runs * fix: address review comments - add UnicodeDecodeError, dict validation, and regression tests - Catch UnicodeDecodeError for invalid UTF-8 encoding - Validate loaded JSON is a dict with required 'run_id' key - Add 5 regression tests for corrupted state files Fixes #3814 --- src/specify_cli/workflows/engine.py | 9 ++- tests/test_workflows.py | 88 +++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 2 deletions(-) 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/test_workflows.py b/tests/test_workflows.py index b9cfd67c0a..8ca7dca50d 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -7332,6 +7332,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 ===== From 0ecb277f0e94387396a0f1fde9411855c7a444dc Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Wed, 5 Aug 2026 18:37:12 +0500 Subject: [PATCH 02/20] fix: skip corrupted run state files in list_runs (#3817) * fix: skip corrupted run state files in list_runs * fix: address review comments - add UnicodeDecodeError, dict validation, and regression tests - Catch UnicodeDecodeError for invalid UTF-8 encoding - Validate loaded JSON is a dict with required 'run_id' key - Add 5 regression tests for corrupted state files Fixes #3817 From 4d5458c8839f1e6d2a9f05c9ec899492ed7d1da6 Mon Sep 17 00:00:00 2001 From: deborre Date: Wed, 5 Aug 2026 14:38:26 +0100 Subject: [PATCH 03/20] fix: keep long frontmatter values on a single line (#3989) `CommandRegistrar.render_frontmatter` calls `yaml.dump()` without `width=`, so PyYAML applies its default ~80-column wrap and folds any long scalar onto a continuation line. A `description` longer than roughly 80 characters is therefore rendered as: --- name: speckit-implement description: Execute the implementation plan by processing and executing all tasks defined in tasks.md --- The YAML remains valid and round-trips faithfully through `yaml.safe_load`, so this is not data loss. It is a shape inconsistency with real consequences: - Hand-written core command templates always keep `description` on one line, so preset- and extension-rendered commands do not match the files they sit beside in the same directory. - Consumers that read frontmatter line-wise rather than with a YAML parser see the description truncated at the fold, followed by a stray line. Spec Kit itself hand-builds SKILL.md frontmatter in the skills path (see #3391), so this is not a hypothetical class of consumer. - `speckit.implement`'s own description is 89 characters, so a preset that overrides it hits this immediately. `width=float("inf")` disables the line-wrapping only; escaping, quoting and the handling of genuinely multi-line values are unchanged, since PyYAML selects the scalar style before applying width. Adds a regression test that fails without the change. Verified against the repo's own suite: 6354 passed. Four failures in tests/integrations/test_integration_subcommand.py are present on a clean checkout too (ANSI escapes in captured output) and are unrelated. --- src/specify_cli/agents.py | 6 +++++- tests/test_extensions.py | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 173f843e42..dede50e0b1 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/tests/test_extensions.py b/tests/test_extensions.py index 616a1dfe12..9442f0bfbe 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 @@ -2996,6 +2997,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 From 6fa8c9aaef7eb7e723883c00b7312a9d786ffdee Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 5 Aug 2026 08:40:04 -0500 Subject: [PATCH 04/20] chore: release 0.16.0, begin 0.16.1.dev0 development (#3992) * chore: bump version to 0.16.0 * chore: begin 0.16.1.dev0 development --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- CHANGELOG.md | 25 +++++++++++++++++++++++++ pyproject.toml | 2 +- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aeec8c726a..26a1f3bea1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,31 @@ +## [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/pyproject.toml b/pyproject.toml index 16811c0e2f..593f5fa218 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "specify-cli" -version = "0.15.3.dev0" +version = "0.16.1.dev0" description = "Specify CLI, part of GitHub Spec Kit. A tool to bootstrap your projects for Spec-Driven Development (SDD)." readme = "README.md" requires-python = ">=3.11" From f01cac630066a5c2f00a81da5be20e423c22395b Mon Sep 17 00:00:00 2001 From: Ali jawwad <33836051+jawwad-ali@users.noreply.github.com> Date: Wed, 5 Aug 2026 18:44:43 +0500 Subject: [PATCH 05/20] fix(scripts): stop setup-tasks text mode crashing on a legacy code page (#3892) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _check_file/_check_dir hard-code U+2713/U+2717 and print() them to sys.stdout. On Windows sys.stdout falls back to the ANSI code page whenever stdout is not a console — which is every time an agent or a workflow step captures the output — and U+2713 is unencodable in cp1252, so the document listing aborted mid-report with UnicodeEncodeError. This is the byte-identical twin of the block in scripts/python/check_prerequisites.py, which I flagged in the PR for that file rather than widening its scope. Fall back to ASCII when stdout cannot encode the glyph. "[OK]"/"[FAIL]" is the rendering these markers already have in-tree: Test-FileExists in scripts/powershell/common.ps1 emits exactly those, and normalize_status_text in tests/parity_helpers.py maps the glyphs onto them. Co-authored-by: Claude Opus 5 (1M context) --- scripts/python/setup_tasks.py | 25 +++++++++++++++++++++---- tests/test_setup_tasks_python_parity.py | 25 +++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) 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/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 From 71125fc3461778b4df1b543bb4d365cab35e2763 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Wed, 5 Aug 2026 19:32:05 +0500 Subject: [PATCH 06/20] test(integrations): guard multiline/control-char SKILL.md frontmatter escaping (#3392) Add regression tests for SkillsIntegration mixin that verify: - Multiline (block-scalar) description round-trips byte-for-byte - C0/DEL control characters in description survive YAML escaping Tests properly isolate Path.home() for Hermes to prevent overwriting a developer's real global skill directory. Refs: #3392 --- .../test_integration_base_skills.py | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/tests/integrations/test_integration_base_skills.py b/tests/integrations/test_integration_base_skills.py index 25551e1dc7..1f29e12227 100644 --- a/tests/integrations/test_integration_base_skills.py +++ b/tests/integrations/test_integration_base_skills.py @@ -141,6 +141,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.""" i = get_integration(self.KEY) From e9710ae45e3c60ce40824b6cf421525d69dcc39e Mon Sep 17 00:00:00 2001 From: Noor ul ain Date: Wed, 5 Aug 2026 20:57:51 +0500 Subject: [PATCH 07/20] fix(archives): wrap the bare EOFError a truncated tar.gz raises (#3938) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(archives): wrap the bare EOFError a truncated tar.gz raises `tarfile` wraps most decompression failures in `TarError`, but a gzip stream that ends before its end-of-stream marker escapes as a bare `EOFError` from the gzip layer. `EOFError` derives from neither `TarError` nor `OSError`, so it bypassed all three of the tar handlers added with tar archive support (#3874): - the format probe in `detect_archive_format`, which caught only `tarfile.TarError`; - `tarfile.open` in `safe_extract_tar`; - member iteration in `safe_extract_tar`. A truncated `.tar.gz` — an interrupted download, a partially written file — therefore raised a raw `EOFError` straight through the caller's `error_type`, so callers catching `ValueError`/`ExtensionError`/ `PresetError` never saw it. In `specify workflow add` the effect is worse than a traceback: Typer treats a bare `EOFError` as a Ctrl-D abort, so the command printed only "Aborted." with no diagnostic at all. The ZIP twin reports "Invalid workflow archive: Invalid ZIP archive: ". Route all three sites through a shared `_TAR_DECOMPRESSION_ERRORS` tuple so they stay in sync. `zlib.error` is included alongside `EOFError`: it is likewise neither a `TarError` nor an `OSError` and can surface from a corrupt deflate block. `OSError` is kept only on the two `safe_extract_tar` sites, which report genuine I/O failures; adding it to the probe would silently swallow them instead. Truncated tar.gz now reports the same clean, domain-typed error as the ZIP path. Tests cover both the short prefix that fails in `tarfile.open` and the longer ones that fail during member iteration — `tarfile` decompresses lazily, so the leak surfaced at different sites depending on how much of the stream survived. Assisted-by: Claude Opus 5 (1M context) Co-Authored-By: Claude Opus 5 (1M context) * test(archives): cover the bare zlib.error a corrupt deflate block raises Review feedback: the `zlib.error` arm of `_TAR_DECOMPRESSION_ERRORS` was not exercised. Every regression added with the fix truncates a valid deflate stream, which raises `EOFError`, so `zlib.error` could regress independently of the EOF handling. It is genuinely reachable, but only under a narrower condition than the truncation cases. `tarfile` converts `zlib.error` to `ReadError` while reading a member *header*, but the forward seek it performs to skip member *data* (`tarfile.next`) sits outside that conversion, so a corrupt region past the first header escapes raw. Reaching that seek needs members larger than the gzip read buffer: with small members the whole stream is decompressed during the first header read and the error is wrapped. The new fixture therefore uses two 256 KiB members at `compresslevel=1` — a ~7 KiB archive — corrupted past the midpoint so the first header still reads clean. Adds four tests: the two `safe_extract_tar` sites (plain and with a caller-supplied `error_type`), the `safe_extract_archive` entry point with a caller-supplied `error_type`, and a guard asserting the fixture still reaches the module as a bare `zlib.error` — so if a future Python wraps it, that fails loudly instead of the coverage silently decaying into a duplicate of the `EOFError` cases. Verified test-the-test: the three wrapping tests fail against the unmodified `_download_security.py` with a raw `zlib.error: Error -3 while decompressing data: invalid distance code`, and pass with the fix. Also corrects the scope claimed for the probe site. Fuzzing 2800 corrupt archives never produced a bare `zlib.error` from `tarfile.open` alone, because the only read it performs is the header read that `tarfile` already converts. The probe's `zlib.error` arm is defensive, not load-bearing; the tuple comment and a detection test now say so rather than implying coverage that cannot exist. Assisted-by: Claude Opus 5 (1M context) Co-Authored-By: Claude Opus 5 (1M context) * test(archives): make the corrupt-deflate fixture zlib-version independent CI failure on macos-latest/3.13: `test_corrupt_deflate_fixture_raises_bare_zlib_error` failed with `gzip.BadGzipFile: CRC check failed`. The other five pytest jobs were fail-fast cancellations, not real failures, and ruff was already green. The fixture built its corruption by XOR-ing 64 arbitrary bytes mid-stream. Whether that produces a *structural* deflate error is zlib-version dependent: on the macOS runner the mangled bytes still decoded, so the stream instead failed the trailing gzip CRC check and raised `BadGzipFile` -- an `OSError`, which the pre-fix `(TarError, OSError)` handler already caught. The guard test exists precisely to catch that degradation, and it did its job. Replaces the XOR with a deflate block header whose `BTYPE` is the reserved value `0b11`. Every zlib rejects that identically as "invalid block type", and it fails during decompression rather than at the CRC check, so no version can turn it into a `TarError` or `OSError`. The stream is assembled by hand (`compressobj(-15)` + explicit gzip header/trailer) so the invalid block lands a controlled 256 KiB into the first member's data -- past the gzip read buffer, so the first header still reads clean and the failure surfaces from the forward seek in `tarfile.next`, which is the site the raw `zlib.error` escapes from. A sweep over clean-prefix sizes confirms a wide margin: with 512 KiB members every prefix from 160 KiB up yields a bare `zlib.error`, versus the transition below ~131 KiB where `tarfile` still wraps it as `ReadError`. The hand-built gzip header also zeroes the mtime field, so the fixture is now byte-identical across builds instead of embedding a timestamp. Strengthens the guard to assert what the fix actually depends on -- that the exception is neither a `TarError` nor an `OSError` -- so the fixture cannot silently decay into an already-caught type again. Production code is unchanged from ef49acc; this is test-only. Verified test-the-test by dropping the `zlib.error` arm from `_TAR_DECOMPRESSION_ERRORS`: the three wrapping tests fail with the raw `zlib.error: Error -3 while decompressing data: invalid block type`, and pass with it restored. `tests/test_download_security.py`: 193 passed. `ruff check src tests` (the exact CI command): all checks passed. Assisted-by: Claude Opus 4.8 (1M context) Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- src/specify_cli/_download_security.py | 24 +++- tests/test_download_security.py | 188 ++++++++++++++++++++++++++ 2 files changed, 209 insertions(+), 3 deletions(-) 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/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}" From f8a448f0a964836186d0dd5af035eb55f0c521ea Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Wed, 5 Aug 2026 21:07:10 +0500 Subject: [PATCH 08/20] fix(skills): apply the line-anchored delimiter scan to hermes and kimi (#3739) Hermes overrides SkillsIntegration.setup() with its own copy of the frontmatter parse and body strip, and Kimi's _is_speckit_generated_skill() parses frontmatter independently, so all three carried the same split("---", 2) bug the base class just fixed. A description such as "Separate sections with --- markers" truncates the parsed frontmatter at the embedded marker, dropping later keys and spilling the remainder into the body; for Kimi that means a Speckit-generated skill is no longer recognized on teardown and gets left behind. Scan for a closing "---" on its own line instead. The body slice keeps whatever trails the marker so output stays byte-for-byte identical for well-formed templates. --- .../integrations/hermes/__init__.py | 44 ++++++++++--- src/specify_cli/integrations/kimi/__init__.py | 16 ++++- .../test_skill_frontmatter_quoting.py | 63 +++++++++++++++++++ 3 files changed, 113 insertions(+), 10 deletions(-) diff --git a/src/specify_cli/integrations/hermes/__init__.py b/src/specify_cli/integrations/hermes/__init__.py index 63ea5f9986..a82eb6fd4d 100644 --- a/src/specify_cli/integrations/hermes/__init__.py +++ b/src/specify_cli/integrations/hermes/__init__.py @@ -121,13 +121,27 @@ def setup( command_name = src_file.stem # e.g. "plan" skill_name = f"speckit-{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: @@ -143,10 +157,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/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 From f31b2b45eb74408f85353b00504c2ff46b159278 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 5 Aug 2026 11:23:18 -0500 Subject: [PATCH 09/20] Fix init-force-preset-desync: reapply presets/extensions on init --here --force (#3995) Apply the remediation from the bug assessment on issue #3990. After integration setup() and manifest.save(), when --force is used (re-initializing an existing project), call _register_presets_for_agent and _register_extensions_for_agent so that previously-installed presets and extensions are recomposed on top of the freshly-regenerated core files. Without this, preset-composed files reverted to pure core while the preset registry continued to report them as installed. This mirrors the same pattern already present in integration_upgrade() (added in PR #3853 / issue #3849 for the upgrade path). Refs #3990 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/commands/init.py | 24 +++++++++ tests/integrations/test_cli.py | 93 ++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index dc4ba90a98..a300c4bcbe 100644 --- a/src/specify_cli/commands/init.py +++ b/src/specify_cli/commands/init.py @@ -635,6 +635,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, diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index 93cadac694..2bb68129b5 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -1067,6 +1067,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.""" From 3d4f71c90ee74beeab67b292ccd7b92c0af0a591 Mon Sep 17 00:00:00 2001 From: Noor ul ain Date: Thu, 6 Aug 2026 18:15:40 +0500 Subject: [PATCH 10/20] fix(extensions): start fresh on a non-UTF-8 extension registry (#3998) ExtensionRegistry._load() catches json.JSONDecodeError and FileNotFoundError to start fresh on a corrupted or missing registry, but a .registry file with invalid UTF-8 bytes raised UnicodeDecodeError from the text-mode read before JSON parsing began. Because the registry is loaded in __init__, that bare traceback broke every extension command -- `specify extension list` on such a project exits with a raw UnicodeDecodeError instead of the module's clean path. Catch UnicodeDecodeError in the same clause: undecodable bytes are the same corruption class as unparseable JSON, only the exception type differs. OSError stays uncaught on purpose -- the data may be intact on disk, and starting fresh would let a later _save() wipe it. This is the exact twin of the PresetRegistry._load() fix in #3955; the two registries are parallel implementations and only the preset side was corrected. _get_installed_sibling_ids() already worked around this gap locally by catching UnicodeError at its own call site; its comment is updated to reflect that _load() now handles the case itself, with the local catch kept as belt-and-braces against regression. Co-authored-by: Claude Opus 4.8 (1M context) --- src/specify_cli/extensions/__init__.py | 18 +++++++++++------- tests/test_extensions.py | 25 +++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index 2e985a0878..9fa44d3809 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -660,8 +660,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): @@ -4310,11 +4315,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/tests/test_extensions.py b/tests/test_extensions.py index 9442f0bfbe..d668019087 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -1291,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 ===== From fe3732e2688adf9dca69f4e3b6002c018d96bb6d Mon Sep 17 00:00:00 2001 From: Marsel Safin <179933638+marcelsafin@users.noreply.github.com> Date: Thu, 6 Aug 2026 15:16:08 +0200 Subject: [PATCH 11/20] fix(presets): return None for an unreadable layer in resolve_content (#3959) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(presets): return None for an unreadable layer in resolve_content PresetResolver.resolve_content() reads the winning layer (and each composition layer) with a bare read_text(), so a layer file that cannot be read or decoded crashed command registration with a raw OSError/UnicodeDecodeError. The docstring already promises 'Composed content string, or None if not found', and since #3896 collect_all_layers() deliberately tolerates a non-UTF-8 legacy layer — moving the crash here, where both callers (_register_commands and _reconcile_composed_commands) are unguarded. Return None when the winning or base layer cannot be read, treating an unreadable layer like a missing one per the documented contract. Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: cover the base guard and composing-layer read Review follow-up: add an unreadable replace base beneath a valid composing layer, and a mocked-PermissionError composing layer over a valid base, so every new boundary and both exception types are covered. Assisted-by: GitHub Copilot (model: claude-fable-5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/presets/__init__.py | 23 +++++- tests/test_presets.py | 107 ++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index cc98c40146..157bac6c46 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -5576,7 +5576,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). @@ -5586,8 +5586,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: @@ -5625,6 +5635,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 @@ -5668,7 +5680,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/tests/test_presets.py b/tests/test_presets.py index 41305518cd..80f2ddab58 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -11425,6 +11425,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) From 3dff6f1d5069ded92096b275fa164364d5b5c2f7 Mon Sep 17 00:00:00 2001 From: Ali jawwad <33836051+jawwad-ali@users.noreply.github.com> Date: Thu, 6 Aug 2026 18:17:36 +0500 Subject: [PATCH 12/20] fix(scripts): stop check-prerequisites text mode crashing on a legacy stdout code page (#3890) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(scripts): stop check-prerequisites text mode crashing on a legacy code page _check_file/_check_dir hard-code U+2713/U+2717 and print() them to sys.stdout. On Windows sys.stdout falls back to the ANSI code page whenever stdout is not a console — which is every time an agent or a workflow step captures the output — and U+2713 is unencodable in cp1252: stdout encoding: cp1252 UnicodeEncodeError: 'charmap' codec can't encode character '✓' So text mode aborted right after printing "AVAILABLE_DOCS:", losing every per-document line. Fall back to ASCII when stdout cannot encode the glyph. "[OK]"/"[FAIL]" is the rendering these markers already have in-tree: Test-FileExists in scripts/powershell/common.ps1 emits exactly those, and normalize_status_text in tests/parity_helpers.py maps the glyphs onto them, so the twins already treat the two forms as equivalent. Co-Authored-By: Claude Opus 5 (1M context) * test(scripts): cover both status markers in the cp1252 regression Review catch: the fixture left every reported document absent (the empty contracts/ also reports missing), so the test only ever called _status_marker(False). The assertion was `"[OK]" in out or "[FAIL]" in out`, which "[FAIL]" alone satisfied. Proved the hole by mutation: replacing the fallback body with a bare `return "[FAIL]"` — deleting the success branch outright — left the test GREEN. Add research.md so one document is present, and assert both markers explicitly. The strengthened test now kills all three mutations: fallback always "[FAIL]" -> FAILS (was passing) fallback always "[OK]" -> FAILS no fallback at all -> FAILS (the original bug) unmutated -> 12 passed, 8 skipped Missing documents are still present in the fixture, so the failure path stays covered too. Co-Authored-By: Claude Opus 5 (1M context) * fix(scripts): restore the _status_marker ASCII fallback The previous commit on this branch unintentionally reverted the source fix while adding the strengthened test, so the branch carried the test without the implementation it tests. Cause: my local verification script reverted the file for its red run with `git checkout upstream/main -- `, which writes the INDEX as well as the working tree. Restoring the working-tree copy afterwards left main's version staged, and the next commit captured it. Restores the fix from 275663b. Verified: 12 passed / 8 skipped, and the red run (source reverted) produces 1 new-vs-baseline failure. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- scripts/python/check_prerequisites.py | 25 ++++++++-- .../test_check_prerequisites_python_parity.py | 46 +++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) 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/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) From 40037b1aca12fcd0775d1332f863d6a1e85d7c0b Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 6 Aug 2026 09:08:09 -0500 Subject: [PATCH 13/20] feat(init): scaffold managed .specify/.gitignore (#4000) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(init): scaffold managed .specify/.gitignore Write a manifest-tracked `.specify/.gitignore` during shared-infra install so machine-local Spec Kit state stays out of version control while everything else under `.specify/` remains shareable: - `feature.json` — the current-feature pointer, rewritten on every feature switch (per-checkout state, not something to share). - `extensions/*/local-config.yml` — per-machine extension config overrides. The file is routed through the same overwrite/skip/preserve policy as shared templates: `--force` refreshes it, user edits are preserved on re-init, and uninstall removes it via the manifest. Addresses github/spec-kit#2304. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98faefd1-9fc8-48fc-bd25-d4f3ccbb2ab9 * docs: correct .specify/.gitignore uninstall claim The file is tracked in the shared-infra manifest (speckit.manifest.json), not the per-integration manifest that `specify integration uninstall` loads. Shared infrastructure is deliberately preserved on uninstall (see test_uninstall_preserves_shared_infra), so `.specify/.gitignore` is left in place rather than removed. Reword the code comment and core.md note to state the actual behavior; keep the true benefits (force-refresh and preserve-on-edit). Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98faefd1-9fc8-48fc-bd25-d4f3ccbb2ab9 * revert: drop manual CHANGELOG.md edit CHANGELOG.md is auto-generated; do not hand-edit it. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98faefd1-9fc8-48fc-bd25-d4f3ccbb2ab9 * test: add .specify/.gitignore to integration file inventories The complete-file-inventory tests assert an exact match of every file produced by `specify init`. Now that shared infra scaffolds a managed `.specify/.gitignore`, add it to the expected inventories so the exact-match assertions pass on both sh and ps script types. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 98faefd1-9fc8-48fc-bd25-d4f3ccbb2ab9 --- docs/reference/core.md | 2 + src/specify_cli/shared_infra.py | 46 ++++++++ .../test_integration_base_markdown.py | 1 + .../test_integration_base_skills.py | 1 + .../test_integration_base_toml.py | 1 + .../test_integration_base_yaml.py | 1 + tests/integrations/test_integration_cline.py | 1 + .../integrations/test_integration_copilot.py | 3 + .../integrations/test_integration_generic.py | 2 + tests/test_shared_infra_gitignore.py | 103 ++++++++++++++++++ 10 files changed, 161 insertions(+) create mode 100644 tests/test_shared_infra_gitignore.py 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/src/specify_cli/shared_infra.py b/src/specify_cli/shared_infra.py index 1c8d727d73..c8d04c9fd9 100644 --- a/src/specify_cli/shared_infra.py +++ b/src/specify_cli/shared_infra.py @@ -16,6 +16,22 @@ 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 @@ -608,6 +624,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/tests/integrations/test_integration_base_markdown.py b/tests/integrations/test_integration_base_markdown.py index aa906c440d..310a0347de 100644 --- a/tests/integrations/test_integration_base_markdown.py +++ b/tests/integrations/test_integration_base_markdown.py @@ -238,6 +238,7 @@ def _expected_files(self, script_variant: str) -> 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 1f29e12227..d064224014 100644 --- a/tests/integrations/test_integration_base_skills.py +++ b/tests/integrations/test_integration_base_skills.py @@ -484,6 +484,7 @@ def _expected_files(self, script_variant: str) -> list[str]: ".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 8a7344e4b2..5469f1350e 100644 --- a/tests/integrations/test_integration_base_toml.py +++ b/tests/integrations/test_integration_base_toml.py @@ -488,6 +488,7 @@ def _expected_files(self, script_variant: str) -> 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_base_yaml.py b/tests/integrations/test_integration_base_yaml.py index f3e39b24f8..3312dfec07 100644 --- a/tests/integrations/test_integration_base_yaml.py +++ b/tests/integrations/test_integration_base_yaml.py @@ -402,6 +402,7 @@ def _expected_files(self, script_variant: str) -> 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_cline.py b/tests/integrations/test_integration_cline.py index f1abdedc8a..5bd25c7d85 100644 --- a/tests/integrations/test_integration_cline.py +++ b/tests/integrations/test_integration_cline.py @@ -185,6 +185,7 @@ def _expected_files(self, script_variant: str) -> 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 7a680b7dd4..b75eac9714 100644 --- a/tests/integrations/test_integration_copilot.py +++ b/tests/integrations/test_integration_copilot.py @@ -274,6 +274,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", @@ -337,6 +338,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", @@ -847,6 +849,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 202f7ab3dd..fab64a9f0a 100644 --- a/tests/integrations/test_integration_generic.py +++ b/tests/integrations/test_integration_generic.py @@ -342,6 +342,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", @@ -399,6 +400,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/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 From 204d94fdb1781c53a0cdb5cd0f3756f3f1c332b1 Mon Sep 17 00:00:00 2001 From: Noor ul ain Date: Thu, 6 Aug 2026 19:12:45 +0500 Subject: [PATCH 14/20] fix(workflows): handle an unreadable run state in `workflow status` (#3999) `workflow status ` and `workflow resume ` both call `RunState.load()`, and a prior fix aligned them on the FileNotFoundError and ValueError boundaries. `resume` also handles OSError; `status` never gained that handler. So an unreadable `state.json` -- wrong permissions, an I/O error, or a directory sitting where the file belongs -- escapes as a raw traceback with no output at all, while `resume` on the same run prints a clean `Error:` line and exits 1. `state_path.exists()` is True for a directory, so the existing guard passes and `open()` raises. Add the missing `except OSError` next to its siblings, using the same `_escape_markup` + `typer.Exit(1)` shape, and routing through `err` so the message lands on stderr under `--json` and the stdout JSON stream stays parseable. Two regression tests: the end-to-end CLI path (a directory in place of state.json) and the `--json` stderr-routing path. Both fail without the source change. Co-authored-by: Claude Opus 4.8 (1M context) --- src/specify_cli/workflows/_commands.py | 6 +++ tests/test_workflows.py | 51 ++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/specify_cli/workflows/_commands.py b/src/specify_cli/workflows/_commands.py index 78a9174c62..813ba992fb 100644 --- a/src/specify_cli/workflows/_commands.py +++ b/src/specify_cli/workflows/_commands.py @@ -1594,6 +1594,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/tests/test_workflows.py b/tests/test_workflows.py index 8ca7dca50d..f3a42c1c2c 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -16705,6 +16705,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.""" From 4a465431b8c27f4748430bdf3215c74da2d7484c Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Thu, 6 Aug 2026 19:58:19 +0500 Subject: [PATCH 15/20] fix: use missing_ok for temp file cleanup to avoid masking errors (#3803) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(agent-context): recurse for nested plans in Python mtime fallback The Python port's mtime fallback discovered plans with a one-level specs/*/plan.md glob, so a scoped layout created via SPECIFY_FEATURE_DIRECTORY (specs///plan.md) was missed when feature.json is absent — the fallback returned no plan and the managed context section omitted the 'at ' line. The bash and PowerShell twins were already fixed to recurse (#3024); the Python twin was left behind. Switch to specs.rglob('plan.md') with the same symlink-safe containment check the bash twin uses (resolve each candidate and confirm it stays within the project root before ranking by mtime), so a plan reached through a specs/ symlink pointing outside the project is not selected. Adds parity regression tests (vs bash and vs PowerShell) covering a nested specs///plan.md; both fail on the pre-fix one-level glob. Fixes #3733 * test(agent-context): cover symlink containment in the mtime fallback The recursive fallback resolves each candidate before the relative_to() containment check, but nothing exercised that path. Add a parity test for a plan reachable only through a specs/ symlink pointing outside the project: relative_to() is lexical and would accept it, emitting an in-project-looking path for an out-of-project file. Both the bash twin and the Python port skip it, so the "at " line is omitted. Also correct the module docstring, which still described the fallback as scanning specs/*/plan.md one level deep. * fix: use missing_ok for temp file cleanup to avoid masking errors --- .../scripts/python/update_agent_context.py | 37 ++++++--- src/specify_cli/_utils.py | 4 +- src/specify_cli/integrations/manifest.py | 3 +- src/specify_cli/shared_infra.py | 3 +- ...test_update_agent_context_python_parity.py | 76 +++++++++++++++++-- 5 files changed, 99 insertions(+), 24 deletions(-) diff --git a/extensions/agent-context/scripts/python/update_agent_context.py b/extensions/agent-context/scripts/python/update_agent_context.py index fc8894ee14..669ec5bf9d 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/src/specify_cli/_utils.py b/src/specify_cli/_utils.py index 85b659d67b..b623de81af 100644 --- a/src/specify_cli/_utils.py +++ b/src/specify_cli/_utils.py @@ -192,8 +192,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/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/shared_infra.py b/src/specify_cli/shared_infra.py index c8d04c9fd9..3aff73ae49 100644 --- a/src/specify_cli/shared_infra.py +++ b/src/specify_cli/shared_infra.py @@ -278,8 +278,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( diff --git a/tests/extensions/test_update_agent_context_python_parity.py b/tests/extensions/test_update_agent_context_python_parity.py index 969192eef3..36e7fd4557 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") From f71cfafa71ee0a8e5ce6d2588114605b2ee49f85 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Thu, 6 Aug 2026 20:21:02 +0500 Subject: [PATCH 16/20] fix: bound response read in integration catalog fetch (#3812) * fix: bound response read in integration catalog fetch * fix: address review - update FakeResponse for bounded reads and add regression test - Update FakeResponse.read() to accept size parameter for bounded reads - Add test_fetch_rejects_oversized_catalog_response regression test - Verifies _fetch_single_catalog uses MAX_JSON_METADATA_BYTES Fixes #3812 * fix: resolve lint errors and update FakeResponse to support bounded reads - Remove duplicate imports of MAX_JSON_METADATA_BYTES and read_response_limited - Update FakeResponse.read() to accept size argument for read_response_limited - Add offset tracking for proper bounded read behavior Refs: #3812 --- src/specify_cli/integrations/catalog.py | 2 +- .../integrations/test_integration_catalog.py | 211 +++++++----------- 2 files changed, 84 insertions(+), 129 deletions(-) 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/tests/integrations/test_integration_catalog.py b/tests/integrations/test_integration_catalog.py index e8a9029db4..68e8970c42 100644 --- a/tests/integrations/test_integration_catalog.py +++ b/tests/integrations/test_integration_catalog.py @@ -223,33 +223,6 @@ def test_load_catalog_config_rejects_falsy_non_mapping_roots( # --------------------------------------------------------------------------- -class _OversizedResponse: - """Response stub that supports bounded streaming reads for oversized-catalog tests.""" - - def __init__(self, data, url=""): - self._data = json.dumps(data).encode() - self._url = url if isinstance(url, str) else url.full_url - self._pos = 0 - - def read(self, n=-1): - if n < 0: - chunk = self._data[self._pos:] - self._pos = len(self._data) - return chunk - chunk = self._data[self._pos : self._pos + n] - self._pos += len(chunk) - return chunk - - def geturl(self): - return self._url - - def __enter__(self): - return self - - def __exit__(self, *a): - pass - - class TestCatalogFetch: """Tests that use a local HTTP server stub via monkeypatch.""" @@ -260,15 +233,15 @@ 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._pos = 0 - - def read(self, n=-1): - if n < 0: - chunk = self._data[self._pos:] - self._pos = len(self._data) - return chunk - chunk = self._data[self._pos:self._pos + n] - self._pos += len(chunk) + 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): @@ -357,6 +330,68 @@ def test_poisoned_cache_shape_is_dropped_and_refetched(self, tmp_path, monkeypat 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_urlopen(req, timeout=10): + return _OversizedResponse() + + monkeypatch.setattr(_auth_http.urllib.request, "urlopen", fake_urlopen) + + 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)) @@ -432,90 +467,6 @@ def test_invalid_catalog_format(self, tmp_path, monkeypatch): with pytest.raises(IntegrationCatalogError, match="Failed to fetch any integration catalog"): cat.search() - def test_oversized_catalog_response_rejected(self, tmp_path, monkeypatch): - """Response exceeding MAX_JSON_METADATA_BYTES is caught as IntegrationCatalogError. - - The per-entry error is logged as a warning and skipped (not fatal). - When ALL catalogs are oversized, search() raises the aggregate error. - """ - from specify_cli._download_security import MAX_JSON_METADATA_BYTES - - 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) - - # Build a valid catalog dict whose JSON encoding exceeds the limit. - oversized = { - "schema_version": "1.0", - "integrations": {}, - "padding": "x" * (MAX_JSON_METADATA_BYTES + 1), - } - - import specify_cli.authentication.http as _auth_http - - def _oversized_urlopen(req, timeout=10): - url = req if isinstance(req, str) else req.full_url - return _OversizedResponse(oversized, url) - - monkeypatch.setattr(_auth_http.urllib.request, "urlopen", _oversized_urlopen) - - # Both default + community catalogs are oversized → all fail → aggregate error. - # The per-entry IntegrationCatalogError (with "exceeds maximum size") is - # logged as a warning; the aggregate raise has a different message. - with pytest.raises(IntegrationCatalogError, match="Failed to fetch any integration catalog"): - cat.search() - - def test_oversized_catalog_does_not_block_healthy_one(self, tmp_path, monkeypatch): - """When one catalog is oversized, the healthy catalog still returns results.""" - from specify_cli._download_security import MAX_JSON_METADATA_BYTES - - monkeypatch.setenv("HOME", str(tmp_path)) - monkeypatch.setenv("USERPROFILE", str(tmp_path)) - monkeypatch.delenv("SPECKIT_INTEGRATION_CATALOG_URL", raising=False) - specify = tmp_path / ".specify" - specify.mkdir() - - healthy_catalog = { - "schema_version": "1.0", - "integrations": { - "good-agent": { - "id": "good-agent", - "name": "Good Agent", - "version": "1.0.0", - "description": "A healthy integration", - "author": "test-org", - }, - }, - } - oversized_catalog = { - "schema_version": "1.0", - "integrations": {}, - "padding": "x" * (MAX_JSON_METADATA_BYTES + 1), - } - cfg = specify / "integration-catalogs.yml" - cfg.write_text(yaml.dump({"catalogs": [ - {"url": "https://healthy.example.com/catalog.json", "name": "healthy", "priority": 1, "install_allowed": True}, - {"url": "https://oversized.example.com/catalog.json", "name": "oversized", "priority": 2, "install_allowed": True}, - ]})) - cat = IntegrationCatalog(tmp_path) - - import specify_cli.authentication.http as _auth_http - - def _multi_catalog_urlopen(req, timeout=10): - url = req if isinstance(req, str) else req.full_url - if "oversized" in url: - return _OversizedResponse(oversized_catalog, url) - return _OversizedResponse(healthy_catalog, url) - - monkeypatch.setattr(_auth_http.urllib.request, "urlopen", _multi_catalog_urlopen) - - # The oversized catalog is skipped; the healthy catalog's integrations are returned. - results = cat.search() - ids = [r["id"] for r in results] - assert "good-agent" in ids - def test_clear_cache(self, tmp_path): (tmp_path / ".specify").mkdir() cat = IntegrationCatalog(tmp_path) @@ -713,19 +664,23 @@ 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._pos = 0 - def read(self, n=-1): - if n < 0: - chunk = self._data[self._pos:] - self._pos = len(self._data) - return chunk - chunk = self._data[self._pos:self._pos + n] - self._pos += len(chunk) + 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 + def __enter__(self): return self + def __exit__(self, *a): pass From 36a33555bc89968a6ff8963733a10bceaab3c7bf Mon Sep 17 00:00:00 2001 From: Ali jawwad <33836051+jawwad-ali@users.noreply.github.com> Date: Thu, 6 Aug 2026 20:21:32 +0500 Subject: [PATCH 17/20] fix(init): escape user-supplied values in `specify init` output (#3787) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(init): escape user-supplied values in `specify init` output commands/init.py interpolated the project name, --integration/--script values and paths straight into Rich markup f-strings. It was the only CLI command module without escaping -- extensions, presets, workflows and integrations all wrap user-controlled display values already. Two consequences, both reproduced end-to-end through the real CLI: 1. SILENT WRONG OUTPUT. `specify init "proj [v2]"` exits 0 and creates the directory, but the Next Steps panel prints 1. Go to the project folder: cd proj Rich ate `[v2]` as a style tag, so the command the user copy-pastes fails. 2. CRASH AFTER SUCCESS. `specify init "app[/red]x"` creates the project and then dies with MarkupError("closing tag '[/red]' ... doesn't match any open tag") -> exit 1 with a traceback for work that actually completed. Wrap the user-controlled display values in rich.markup.escape: project name (error/warning/conflict/next-steps), project and working paths, the echoed --integration and --script values, and the agent folder in the gitignore hint. Display only -- no control flow, exit codes or messages change, and escape is a no-op for any value without a tag-shaped bracket run. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) * fix(init): shell-quote the project name in the Next Steps cd line Rich-escaping stopped the brackets being swallowed, but the printed command was still unusable for any name containing whitespace: `cd proj v2` is two arguments in every shell. $ cd proj v2 -> /bin/bash: line 1: cd: too many arguments (rc=1) $ cd "proj v2" -> rc=0, lands in "proj v2" Quote it for the host the same way _version._render_argv renders its copy-pasteable installer command: subprocess.list2cmdline on Windows, shlex.quote elsewhere. Windows must use 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. Names needing no quoting are returned unchanged, so the common case is byte-identical. Shell-quote inner, Rich-escape outer. Tests execute the printed command through a real shell rather than only inspecting the string, and pin that an ordinary name stays unquoted. Co-Authored-By: Claude Opus 5 (1M context) * fix(init): drop the now-redundant local escape imports that shadowed the module one Rebasing onto main brought in three new extension-install helpers, and two of them carry a function-local from rich.markup import escape as _escape_markup inside `register > init`. This PR adds the same import at module level, so the locals made `_escape_markup` a local variable for the whole `init` function — every use *before* those import lines then raised UnboundLocalError: cannot access local variable '_escape_markup' where it is not associated with a value which broke `specify init` outright (7 of 8 tests in this file failed after the rebase, all with exit_code 1). The locals are redundant now that the module-level import exists, so remove them. Verified with an AST scope walk that the only remaining `_escape_markup` imports are the module-level one and the one inside `_confirm_extension_url_trust`, which has no module-level use to shadow. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- src/specify_cli/commands/init.py | 56 +++++++--- tests/test_init_output_markup.py | 176 +++++++++++++++++++++++++++++++ 2 files changed, 216 insertions(+), 16 deletions(-) create mode 100644 tests/test_init_output_markup.py diff --git a/src/specify_cli/commands/init.py b/src/specify_cli/commands/init.py index a300c4bcbe..2bb8452025 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 ( @@ -169,6 +172,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: @@ -351,7 +373,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) @@ -428,26 +453,27 @@ 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)" + 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"[cyan]--force supplied: merging into existing directory '[cyan]{project_name}[/cyan]'[/cyan]" + f"[cyan]--force supplied: merging into existing directory '[cyan]{safe_name}[/cyan]'[/cyan]" ) else: error_panel = Panel( - f"Directory already exists: '[cyan]{project_name}[/cyan]'\n" + f"Directory already exists: '[cyan]{safe_name}[/cyan]'\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]", @@ -461,7 +487,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 @@ -500,12 +526,14 @@ def init( setup_lines = [ "[cyan]Specify Project Setup[/cyan]", "", - f"{'Project':<15} [green]{project_path.name}[/green]", - f"{'Working Path':<15} [dim]{current_dir}[/dim]", + f"{'Project':<15} [green]{_escape_markup(project_path.name)}[/green]", + 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]" + ) console.print( Panel("\n".join(setup_lines), border_style="cyan", padding=(1, 2)) @@ -532,7 +560,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 @@ -571,8 +599,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)}" @@ -827,8 +853,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() @@ -918,7 +942,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 [cyan]{agent_folder}[/cyan] (or parts of it) to [cyan].gitignore[/cyan] to prevent accidental credential leakage.", + f"Consider adding [cyan]{_escape_markup(str(agent_folder))}[/cyan] (or parts of it) to [cyan].gitignore[/cyan] to prevent accidental credential leakage.", title="[yellow]Agent Folder Security[/yellow]", border_style="yellow", padding=(1, 2), @@ -929,7 +953,7 @@ def init( steps_lines = [] if not here: steps_lines.append( - f"1. Go to the project folder: [cyan]cd {project_name}[/cyan]" + f"1. Go to the project folder: [cyan]cd {_escape_markup(_shell_quote_arg(str(project_name)))}[/cyan]" ) step_num = 2 else: 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'" From adb2413ab6ef3c2038dc455fea61c5153e4bfc75 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Thu, 6 Aug 2026 20:45:29 +0500 Subject: [PATCH 18/20] fix: add utf-8 encoding to extension and preset registry file I/O (#3834) Both extension and preset registry read/write calls used platform-default encoding, which on Windows (cp1252/UTF-16) would corrupt UTF-8 JSON data or raise UnicodeDecodeError. Explicitly specify encoding='utf-8' to match the JSON contract. Assisted-by: opencode (autonomous) From 21ccf0f746e82512735259b83ce38e19b451b820 Mon Sep 17 00:00:00 2001 From: Lior Kanfi Date: Thu, 6 Aug 2026 23:10:57 +0300 Subject: [PATCH 19/20] docs: add 0.16.0+adlc1 FORK.md entry and refresh AGENTS.md team-boot directive Add version-history row for the 0.16.0+adlc1 upstream merge (18 commits, 4 conflicts resolved, .gitignore scaffolding, catalog mock-target fix). Refresh the AGENTS.md managed Spec Kit section: mandate team-boot skill invocation before any task, add anti-patterns for skipping the skill check, and fix the team-ai-directives constitution path. Assisted-by: opencode (model: glm-5.2, autonomous) --- AGENTS.md | 9 +++++++-- FORK.md | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) 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/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. | From b8685f7c2915639b5162a7f1dbbd8349ed1a70b1 Mon Sep 17 00:00:00 2001 From: Lior Kanfi Date: Fri, 7 Aug 2026 09:02:06 +0300 Subject: [PATCH 20/20] chore: stop preinstalling levelup extension (adlc-team-skills coexistence) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The levelup extension's 6 commands (/levelup.init, /levelup.clarify, /levelup.specify, /levelup.skill, /levelup.implement, /levelup.validate) overlap with adlc-team-skills' levelup-* skills/commands. When both toolkits are used together (the recommended flow: install adlc-team-skills first, run team-setup, then specify init without --team-ai-directives), this creates command redundancy with different naming conventions (dot vs hyphen) for the same lifecycle. Setting levelup.preinstall to false removes it from auto-install while keeping the extension bundled and available on demand via 'specify extension install levelup'. Version bump: 0.16.0+adlc1 → 0.16.0+adlc2 Assisted-by: opencode (model: glm-5.2, supervised) --- CHANGELOG.md | 15 +++++++++++++++ extensions/catalog.json | 2 +- pyproject.toml | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f57b6e2b50..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 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 253f175891..36ae95d9a9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "agentic-sdlc-specify-cli" -version = "0.16.0+adlc1" +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"