Clear pre-existing lint debt flagged by repo-wide `ruff check` (the lint
config only scopes src/, so tests/ had drifted). No behavior change.
- F401/F541: drop unused imports and redundant f-string prefixes (autofix)
- E741: rename ambiguous `l` to `ln` in comprehensions
- E702: split semicolon-joined statements onto separate lines
- F841: drop unused bindings while keeping the side-effecting calls
(_minimal_feature, install_from_directory)
Full suite: 3344 passed, 40 skipped. ruff check (repo-wide): clean.
* fix(workflows): validate run_id in RunState.load before touching the filesystem
``RunState.load(run_id, project_root)`` interpolates ``run_id`` directly
into ``project_root / ".specify" / "workflows" / "runs" / run_id`` and
then calls ``state_path.exists()`` and ``json.load`` on the result. The
run_id is reachable from user input via ``specify workflow resume
<run_id>`` (CLI argument) and via ``SPECKIT_WORKFLOW_RUN_ID`` (env var
override on the engine's run path), so a value like ``../escape``
turns ``runs_dir`` into ``.specify/workflows/escape/`` and:
* ``state_path.exists()`` becomes a file-existence oracle for any
path the process can read.
* if a ``state.json`` exists at the traversed location (planted by
a malicious dependency, a misconfigured shared workspace, or an
older spec-kit version that happened to write there),
``json.load`` parses it and the workflow resumes under the
attacker-chosen ``workflow_id`` / step state.
* a subsequent ``state.save()`` then writes back to the traversed
location, persisting the corruption.
``RunState.__init__`` already validates ``run_id`` against
``r'^[a-zA-Z0-9][a-zA-Z0-9_-]*$'`` — but that check runs on
``state_data["run_id"]`` *after* ``load`` has already done the file
lookup, which is too late to prevent the disclosure.
This change extracts the pattern into a class-level constant
``_RUN_ID_PATTERN`` and a single ``_validate_run_id`` classmethod so
``__init__`` and ``load`` cannot drift, then calls the validator at the
top of ``load`` before any path is built. Mirrors the precedent in
``src/specify_cli/agents.py::_ensure_within_directory`` (used at line
437 of that file) which guards extension-install paths against the
same threat model.
Regression tests parametrize 9 traversal vectors (``../escape``,
``..``, ``../../etc/passwd``, ``foo/bar``, ``foo\bar``, ``.hidden``,
``-flag``, ``foo\x00bar``, empty) and plant a malicious ``state.json``
outside ``runs/`` so a missing guard would surface as a successful
load rather than the ambiguous ``FileNotFoundError``. A second test
asserts ``__init__`` and ``load`` reject the same representative
malformed ID, so future changes to one path can't silently drift from
the other.
* test(workflows): exercise RunState.load in shared-validation test, fix __init__ empty-string asymmetry
Copilot's review on this PR pointed out that
test_init_and_load_share_validation claimed to verify both entry
points share the same validation rules but never actually called
RunState.load — only __init__ and the shared
_validate_run_id helper. A regression in load (e.g. someone
deleting the cls._validate_run_id(run_id) call before the path is
built) would slip through even though __init__ and the helper
stayed aligned, defeating the whole point of the test.
Tightening the test surfaced a real asymmetry the previous version was
silently masking:
self.run_id = run_id or str(uuid.uuid4())[:8]
The truthiness fallback meant RunState(run_id="") silently
substituted a UUID and skipped validation, while
RunState.load("", project_root) correctly rejected the empty
string. The two entry points diverged on the empty-string vector.
That is exactly the drift the test name claimed to defend against —
and the original test missed it.
Changes
-------
* engine.py: __init__ now distinguishes run_id is None
(caller omitted it → auto-generate UUID) from an empty string
(caller provided it → must validate like any other value). Both
paths still flow through _validate_run_id, but only the
explicit-None case auto-generates.
* test_workflows.py: test_init_and_load_share_validation is
now parametrized over one representative vector per category from
test_load_rejects_path_traversal (parent traversal, embedded
separator, leading non-alphanumeric, empty string) and asserts that
*all three* entry points — __init__, _validate_run_id, and
load — reject the same input. Adding load to the assertion
is the substantive fix Copilot asked for; keeping __init__ and
the helper alongside it makes any future drift between the three
immediately observable instead of having to read three separate
tests.
Verification
------------
pytest tests/test_workflows.py — 168 passed (was 165 before the
parametrize expansion; __init__ empty-string vector would have
failed the new test against the old engine code, confirming the
asymmetry was real).
* feat(cli): implement specify self upgrade
* fix(cli): normalize self-upgrade prerelease tags
* fix(cli): tighten self-upgrade diagnostics
* fix(cli): harden self-upgrade verification parsing
* fix(cli): sanitize self-check fallback tags
* fix(cli): harden self-check release display
* fix(cli): validate resolved upgrade tags
* fix(cli): tolerate invalid install metadata
* test(cli): align upgrade network mocks
* fix(cli): respect relative installer paths
* fix(cli): tighten upgrade failure handling
* fix(cli): align installer path diagnostics
* fix(cli): validate release and version output
* fix(cli): clarify source checkout guidance
* fix(cli): harden upgrade detection helpers
* fix(cli): avoid echoing invalid release tags
* fix(cli): tolerate argv path resolve failures
* chore: remove self-upgrade formatting-only diffs
* fix: address self-upgrade review feedback
* fix: address self-upgrade review followups
* fix: address self-upgrade review edge cases
* fix: address self-upgrade review docs
* fix: refine self-upgrade review followups
* fix: address self-upgrade review cleanup
* fix: handle self-upgrade review edge cases
* fix: address self-upgrade review nits
* fix: address follow-up self-upgrade review
* fix: resolve self-upgrade review and Windows CI failures
- README: promote "Optional Commands" to ### so it is a sibling of
"Core Commands" under "Available Slash Commands" (consistent heading
levels; avoids the h2->h4 jump a revert would create).
- _version: allow --tag prerelease/dev and build-metadata suffixes to
compose (e.g. v1.0.0-rc1+build.42), matching PEP 440 / semver; the
Version() check still enforces canonical validity.
- tests: compare resolved argv0 as Path objects instead of POSIX strings
so the assertion holds on Windows; skip the relative-installer-path
executable-bit tests on Windows via a new requires_posix marker (they
rely on chmod/X_OK semantics and chdir-into-tmp teardown that do not
hold there). Add a combined prerelease+build-metadata tag test.
* fix: address second self-upgrade review round
- self_check: clarify that the "up to date" branch is reached only for
parseable latest tags (the unparseable case returns earlier), so the
InvalidVersion fallback assumption is not reintroduced.
- self_upgrade: compare target/current as Version instances directly
instead of re-parsing the canonical strings through _is_newer; the
empty-current case stays explicit via the not-None guard.
- tests: document the intentional broad GH_/GITHUB_ env scrub with a test
asserting non-credential context vars (GH_HOST, GITHUB_REPOSITORY, …) are
stripped from the installer subprocess env — a deliberate fail-safe that
also catches credential-adjacent names without a recognized suffix.
* fix: address third self-upgrade review round
- self_upgrade: unify the no-op short-circuits on packaging Version
equality instead of canonical-string equality. Version("1.0") equals
Version("1.0.0") but their str() forms differ, so the old check could
misreport an equal install as "already on latest release or newer".
Both the unpinned and pinned branches now use Version comparison.
- self_upgrade: compare the verified version as a parsed Version against
the target so a non-version verifier result is a mismatch (exit 2)
rather than a coincidental canonical-string match.
- resolver: map HTTP 429 (Too Many Requests / secondary rate limit) to
the rate-limited category so users get the same actionable token hint
as 403.
- _is_github_credential_env_key: document the precise (intentionally
broad) scrub matching contract in the docstring.
- tests: add a trailing-zero Version-equality regression test and a
parametrized HTTP-status categorization test (429 -> rate limited;
404/502 -> verbatim).
* fix: address fourth self-upgrade review round
- self_upgrade: label a pinned target older than the installed version as
"Downgrading" rather than "Upgrading" so `--tag <older>` is not mistaken
for a forward upgrade.
- resolver: drop the unused `typing.Optional` import and annotate the
`--tag` option as `str | None`, consistent with the rest of the module
(verified Typer resolves it on the supported Python versions).
- _is_github_credential_env_key: add `_PASSWORD` and `_CREDENTIALS` to the
recognized credential suffixes and document that only these shapes are
scrubbed (not blanket coverage).
- tests: assert the precise exit code (1) for the re-raised transient
OSError path; skip the InvalidMetadataError test on Pythons where the
real exception is absent instead of fabricating it; update the pinned
downgrade test to expect the "Downgrading" label.
* fix: accept uppercase V prefix in --tag
Fold a leading uppercase `V` (a common paste) to the canonical lowercase
`v` before validating `--tag`. The remainder of the tag stays
case-sensitive on purpose: the validated value is used verbatim as a git
ref, which is case-sensitive on GitHub, so rewriting label/build-metadata
casing could point at a tag that does not exist. Adds a normalization test.
`workflow resume` now accepts `--input key=value` (the same flag and
parsing as `workflow run`, via a shared `_parse_input_values` helper).
Supplied values are merged over the run's persisted inputs and
re-resolved through the existing typed-validation path
(`_resolve_inputs`), so a resumed/re-run step sees the updated inputs
and ill-typed values fail fast. Keys not supplied keep their persisted
values; resuming without `--input` is unchanged. Reference docs updated.
Distinct from #2405 (file-reference inputs at run time): this is about
supplying inputs at resume time, reusing the existing input model.
Closes#2812.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
On Windows, when stdout/stderr are not a UTF-8 TTY (output piped, redirected
to a file, or running under a legacy code page such as cp1252), Rich cannot
encode the banner and box-drawing glyphs, so the CLI aborts with a
UnicodeEncodeError traceback instead of printing. This breaks basic commands
like `specify --help` and `specify version` whenever their output is captured
rather than written to an interactive terminal.
Reconfigure sys.stdout/sys.stderr to UTF-8 with errors="replace" at the
main() entry point on win32 so output degrades gracefully instead of crashing.
The change is a no-op on POSIX, is guarded by try/except so it can never make
stream setup worse, and lives at the CLI entry point only -- importing
specify_cli as a library does not touch global streams.
Verified on Windows 11 (cp1252): `specify --help` piped and `specify version`
redirected to a file both render correctly and exit 0 without setting
PYTHONUTF8 / PYTHONIOENCODING.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* chore: bump version to 0.9.2
* chore: begin 0.9.3.dev0 development
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* fix: resolve GitHub release asset API URL for private repo downloads
For private or SSO-protected GitHub repos, browser release download URLs
redirect to HTML/SSO instead of the ZIP asset. This commit resolves the
asset via the GitHub REST API and downloads with Accept: application/octet-stream,
falling back to the original URL if the API call fails.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: support direct GitHub REST release asset URLs in extension downloads
When a catalog download_url is already a GitHub REST release asset URL
(https://api.github.com/repos/<owner>/<repo>/releases/assets/<id>),
skip the release metadata lookup and download directly with
Accept: application/octet-stream. This complements the browser URL
resolution from the previous commit, covering catalogs that reference
the REST API directly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
VS Code Copilot Agent Skills do not support the `mode:` frontmatter field.
The generated SKILL.md files included `mode: speckit.<stem>` injected by
CopilotIntegration.post_process_skill_content(), which had no effect in
VS Code and could cause confusion. Simplify post_process_skill_content to
delegate directly to _CopilotSkillsHelper without injecting mode:.
Update tests to assert mode: is absent from generated skill frontmatter.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(integrations): co-locate integration commands in integrations/ domain dir
- Remove commands/ stubs (handlers will live in domain dirs)
- Move all integration CLI handlers out of __init__.py into integrations/
- Split into focused modules under integrations/:
_helpers.py (340 lines) — domain helpers
_install_commands.py (306 lines) — install / uninstall
_migrate_commands.py (487 lines) — switch / upgrade
_query_commands.py (442 lines) — list / use / search / info / catalog
_commands.py (34 lines) — app objects + register()
- __init__.py reduced by ~1400 lines; integration block replaced with register() call
- Fix patch paths in tests to new module locations
* fix(integrations): restore original integration list output in refactor
Preserve the CLI Required column, post-table default/installed summary,
and no-installed guidance that were dropped during the no-behavior-change
refactor of integration list into _query_commands.py.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix(integrations): restore _clear/_update_init_options public imports
The refactor that split integration commands moved
_clear_init_options_for_integration and _update_init_options_for_integration
into integrations/_helpers.py, but tests still import them from the top-level
specify_cli package, causing ImportError. Re-export them with explicit aliases
at the end of __init__.py to preserve the public import surface.
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* feat(workflows): add continue_on_error step field
Adds an optional `continue_on_error: bool` field on every step.
When set to `true` and the step fails, the engine records the
result (`exit_code`, `stderr` on `steps.<id>.output` plus `status`
as a sibling key on `steps.<id>`) and continues to the next sibling
step instead of halting the run. Downstream `if`, `switch`, or
`gate` steps can then branch on
`{{ steps.<id>.output.exit_code }}` to route the recovery path.
Engine details
--------------
`WorkflowEngine._execute_steps` now consults the step config when a
step returns `StepStatus.FAILED`:
- Gate aborts (`output.aborted`) always halt the run — operator
decisions take precedence over the flag.
- Otherwise, if `continue_on_error` is the literal `True`, log a
`step_continue_on_error` event and proceed to the next sibling.
The runtime check uses identity comparison (`is True`) rather
than truthiness, so truthy non-bool values like the string
`"true"` cannot silently change run semantics even if a caller
bypasses `validate_workflow()`.
- Otherwise, behave as before: log `step_failed`, set
`RunStatus.FAILED`, and return.
Validation
----------
`_validate_steps` rejects non-bool values for `continue_on_error`.
Coerced strings like `"true"` are not accepted so authoring
mistakes surface at validation time rather than silently changing
run semantics.
Tests
-----
`TestContinueOnError` in `tests/test_workflows.py` (8 tests):
- `test_undeclared_failure_halts_run` — default halt behaviour.
- `test_declared_and_fired_continues_run` — flag + fail → continue.
- `test_declared_but_step_succeeded_is_noop` — flag + success → no-op.
- `test_if_branch_routes_around_failure` — end-to-end recovery.
- `test_gate_abort_still_halts_with_continue_on_error` — abort
always halts.
- `test_validation_rejects_non_bool_continue_on_error` — `"true"`
rejected at validation.
- `test_validation_accepts_bool_continue_on_error` — `true`/`false`
pass cleanly.
- `test_engine_ignores_truthy_non_bool_continue_on_error` —
defense-in-depth: engine ignores string `"true"` even when
validation is bypassed.
Rebased onto current upstream/main (post #2664 merge); the new
`TestContinueOnError` class sits immediately after upstream's
`TestContextRunId` so the two feature suites coexist cleanly.
Closes#2591.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(workflows): restore runtime context section, clarify gate prompt
Two Copilot findings on d0b9e00:
1. The `### Runtime Context` documentation for `{{ context.* }}` was
lost during the rebase onto current main (the squash dropped the
anchor where #2664 had added it). Restored under `## Expressions`
so users can find `context.run_id` semantics and examples.
2. The continue_on_error example gate had message "Retry or skip?"
but used the default `options: [approve, reject]` with `on_reject:
skip`, which implied an automatic retry path that gates do not
provide. Reworded the message to match the actual approve/reject
semantics and added an explicit note that retry requires either
custom gate options + downstream branching or a wrapper loop.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(workflows): clarify continue_on_error scope — returned FAILED only
Copilot finding on d0b9e00:
The README's "Error Handling" intro implied `continue_on_error` covers
"any other runtime error raised during step execution", but the engine
only consults the flag when a step returns `StepResult(status=FAILED, ...)`.
Exceptions raised out of `step_impl.execute()` propagate to
`WorkflowEngine.execute()`, where the catch-all logs `workflow_failed`
and re-raises — the step result is never recorded, and the flag is
never consulted.
Audited the whole PR diff for the same overclaim:
1. workflows/README.md — main fix. Reworded the Error Handling intro to
"any step that returns StepResult(status=FAILED, ...)" and promoted
the parenthetical structural-validation note into the Notes block.
Added a new "Scope: returned failures only" note that names the
exception path explicitly and tells step authors how to bring the
flag into scope for exceptional code (catch internally and return
FAILED with the failure encoded in `output`).
2. tests/test_workflows.py — section comment used "when an executable
step fails", same ambiguity. Tightened to "when a step returns
StepResult(status=FAILED, ...)" and added a sentence calling out
that unhandled exceptions are out of scope.
3. src/specify_cli/workflows/engine.py — already correct ("any step
that returns FAILED" in the validator comment; "lets the pipeline
route around the failure" in the execute path). No change.
Engine semantics and test bodies are unchanged. Docs-only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(workflows): clarify on_reject:skip semantics — engine returns COMPLETED, not auto-skip
Copilot finding on b8982a7:
The README example's gate message said "reject to skip the rest of this
branch", and the explanatory paragraph claimed [approve, reject] map
to "continue" vs "skip the rest of this branch". The engine does not
implement automatic branch-skipping. `on_reject: skip` returns
`StepStatus.COMPLETED` (gate/__init__.py:65-66); the next sibling step
runs unconditionally unless the author wires a downstream `if` reading
`{{ steps.<gate-id>.output.choice }}`.
Two fixes:
1. Restructured the YAML example so it actually demonstrates the
manual-branching pattern: added a `recover` if-step after the gate
that conditions on `steps.review.output.choice == 'approve'`. Now
the example shows the real workflow author's responsibility instead
of implying the engine does it.
2. Replaced the trailing paragraph with three precise notes:
- both gate options return COMPLETED; `on_reject: skip` controls
abort behaviour only, not sibling-skipping
- all three `on_reject` values enumerated with their actual engine
semantics (FAILED+aborted / COMPLETED / PAUSED)
- the original retry-loop guidance retained as the third bullet
Updated the gate message in the example to match — "reject to leave the
failure recorded and move on" instead of "reject to skip the rest of
this branch".
Audited the whole PR diff for the same overclaim: no other instance.
Engine semantics, validation, and test bodies are unchanged. Docs-only.
161/161 tests/test_workflows.py pass locally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(workflows): clarify gate's role — surfaces, doesn't programmatically branch
Audit follow-up to 393ac6b — three sites repeated the same minor
overclaim about gates being one of the "branch on it" step types
alongside `if` and `switch`:
1. workflows/README.md (the "downstream `if`, `switch`, or `gate`
steps can branch on it" sentence introducing the example)
2. engine.py:236 (validator inline comment)
3. engine.py:657 (execute-path inline comment)
A `gate` step does not have a `condition` or `expression` field — it
only evaluates expressions for `message` and `show_file` (gate/__init__.py:29,36).
Programmatic branching happens in `if`/`switch`; a gate surfaces the
value to a human operator via message interpolation, and the operator's
choice is recorded in `output.choice` for a *subsequent* `if`/`switch`
to route on.
Reworded all three sites consistently: "a downstream `if` or `switch`
can branch on it (or a `gate` can surface it to the operator via
message interpolation)". The README example already demonstrates this
distinction — the gate carries `{{ }}` template variables in its
message and the `recover` if-step downstream is what actually branches
on the choice.
Engine semantics, validation, and test bodies are unchanged. Docs-only
on the README; comment-only on engine.py.
161/161 tests/test_workflows.py pass locally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(workflows): use qualified StepStatus.* instead of bare FAILED/COMPLETED/PAUSED
Three Copilot inline comments on workflows/README.md lines 226, 282, 288
flagged that ``StepResult(status=FAILED, ...)`` is not valid Python —
``StepResult.status`` is a ``StepStatus`` enum value, so the
documented form should be ``StepStatus.FAILED``.
Audited the whole PR diff for the same shorthand. The bare unqualified
form appears in three files added/modified by this PR:
1. workflows/README.md (6 sites) — three ``StepResult(status=FAILED, ...)``
parentheticals, plus the on_reject Notes bullet listing the three
step statuses (``FAILED``, ``COMPLETED``, ``PAUSED``).
2. tests/test_workflows.py (4 sites) — section header for
TestContinueOnError, two test-method docstrings, one inline comment
about a gate's TTY-fallback behaviour.
3. src/specify_cli/workflows/engine.py (1 site) — the validator inline
comment added in d0b9e00 said "returns FAILED" where the engine
code itself uses ``StepStatus.FAILED``.
All 11 sites normalised to the qualified ``StepStatus.<name>`` form so
the docs / test docstrings / inline comments match what readers will
actually find in the engine code and the tests. Engine semantics,
validation, and test bodies are unchanged.
161/161 tests/test_workflows.py pass locally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(shared-infra): record skipped files in speckit.manifest.json
`install_shared_infra` skipped files that already existed on disk
when `force=False`, but the skip branches in both the scripts loop
and the templates loop only appended to `skipped_files` without
calling `manifest.record_existing`. So when the function ran with a
fresh manifest against an already-populated `.specify/` tree (e.g.
after the manifest was deleted, corrupted, or extracted out of band),
every file went down the skip path, `planned_copies` /
`planned_templates` stayed empty, and `manifest.save()` wrote an
empty `files` field — leaving the integration believing nothing was
installed.
Record every skipped file in the manifest, but only when it is not
already tracked. This preserves the original hash for files that
were previously recorded so `check_modified()` (used by
`integration use` to decide whether a user has customized a
template) keeps working correctly.
Add `TestSpeckitManifestRecordsSkippedFiles` in
`tests/integrations/test_integration_claude.py` covering both the
fresh-skip path and the recover-after-lost-manifest path.
Fixes#2107
* fix(shared-infra): guard manifest.record_existing against non-file dst
Address Copilot review feedback on PR #2483. The previous fix called
``manifest.record_existing(rel_skip)`` from the skip branch of both
loops in ``install_shared_infra``, which would crash with
``IsADirectoryError`` (or another ``OSError``) if a directory or other
non-regular-file happened to exist at the expected destination path —
since ``record_existing`` opens the file to compute its SHA-256.
Three coordinated fixes:
1. ``IntegrationManifest.record_existing`` now validates its
precondition: it raises ``ValueError`` if the path is a symlink or
is not a regular file. The docstring already promised "an
already-existing file"; this enforces it. The symlink check runs on
the un-resolved path because ``_validate_rel_path`` calls
``resolve()``, which would silently follow the symlink. Mirrors the
existing ``_ensure_safe_manifest_destination`` precedent in the
same module.
2. In ``install_shared_infra``'s scripts and templates skip branches,
guard the ``record_existing`` call with ``dst.is_file()`` and wrap
it in ``try/except (OSError, ValueError)``. A directory collision,
permission error, or TOCTOU race no longer aborts the whole
install — the user gets a per-path warning, the path still
surfaces in ``skipped_files``, and the rest of the install
continues.
3. ``_read_manifest_files`` in the regression test no longer falls
back to ``data.get("_files")`` (Copilot's low-confidence finding):
the silent fallback could mask a schema regression where the
public ``files`` key is renamed. It now asserts ``"files" in data``
and that the value is a dict.
Add two regression tests in ``TestSpeckitManifestRecordsSkippedFiles``
covering the directory-at-destination edge case for both the scripts
loop and the templates loop. Both verify (a) install does not crash,
(b) the non-file path is not recorded in the manifest, and (c) the
path still surfaces in the user-visible warning.
The "shared infrastructure file(s)" warning text is changed to
"path(s)" so it remains accurate when non-file entries appear in the
list.
Refs #2107
* fix(manifest): lexical pre-check for record_existing + add error-case tests
Address Copilot review (2026-05-11, review id 4266902103):
1. `record_existing` was calling `(self.project_root / rel).is_symlink()`
BEFORE validating containment. For absolute paths or paths containing
`..`, this performed a filesystem stat outside the project root before
`_validate_rel_path()` raised. Add a cheap lexical pre-check that
delegates to `_validate_rel_path()` for the canonical error messages,
so the symlink stat only ever runs on paths that are already lexically
inside the project root.
2. Add focused unit tests in `tests/integrations/test_manifest.py` for
the symlink and non-regular-file error paths, including:
- symlink target rejection
- dangling symlink rejection (caught by the symlink guard before
the is_file check)
- directory path rejection (is_file == False)
- missing-path rejection (is_file == False)
- absolute-path lexical pre-check
The Copilot reviewer noted these guards had no focused coverage in
`test_manifest.py`, only via the `test_integration_claude.py`
regression test.
3. The third Copilot finding (repeated `dict(self._files)` copies via
`manifest.files` in the skip branches) is already resolved on this
branch by using `prior_hashes` — the function-scope snapshot taken at
the top of `install_shared_infra` — for the membership check, instead
of `manifest.files`.
AI disclosure: drafted with assistance from Claude (Opus 4.7).
* fix(manifest): track recovered files separately + symlink-ancestor + canonical-path guards
Address Copilot review id 4309888722 (2026-05-18) on PR #2483:
1. Recovery semantics (shared_infra.py:371, 412) — install_shared_infra
now passes ``recovered=True`` when re-recording a skipped existing
file. This flag funnels into a new ``recovered_files`` array in the
manifest JSON, so a future ``refresh_managed`` run can distinguish
"hash I produced" from "hash I observed on a file that may be a user
customization" and avoid silent overwrite without ``--refresh-shared-infra``.
Schema is purely additive: ``files: dict[str, str]`` is unchanged; the
new ``recovered_files: list[str]`` is omitted when empty.
2. Symlinked ancestor (manifest.py:172) — ``record_existing`` now walks
every component of the rel path and rejects any symlinked ancestor,
not just a symlinked leaf. Catches ``linked_dir/file.txt`` where
``linked_dir`` is a symlink, which previously slipped past the leaf-only
``is_symlink()`` check and was resolved through by ``_validate_rel_path``.
Mirrors the component-walk pattern in ``_ensure_safe_manifest_directory``.
3. Misleading "escapes project root" message (manifest.py:168) — paths
like ``dir/../file.txt`` normalize inside the project, so the old
message lied about what was wrong. New message: "Manifest paths must
be canonical; '..' segments are not allowed". Still rejects (canonical
keys are required so ``check_modified``/``uninstall`` cannot key the
same file under two paths).
Tests: 7 new test methods across TestManifestRecoveredFiles and
TestRecordExistingNewGuards covering all 4 Copilot findings. Full suite
passes locally.
🤖 AI disclosure: drafted with assistance from Claude (Opus 4.7).
* fix(manifest): normalize is_recovered input through _validate_rel_path
Address Copilot review comment id 4309888722 round-5 (2026-05-21) on PR #2483:
``is_recovered()`` previously checked ``self._recovered_files`` membership
with bare ``Path(rel).as_posix()``, while ``record_existing()`` stores keys
via ``_validate_rel_path(rel, root).relative_to(root).as_posix()``. The two
normalizations disagreed on absolute paths and paths that escape the
project root — ``is_recovered`` would silently return False for inputs that
``record_existing`` would have refused entirely.
The fix routes ``is_recovered`` through the same ``_validate_rel_path``
pipeline; ``ValueError`` from the validator is caught and converted to
False so query semantics stay exception-free (Python ``__contains__``
convention).
Tests: 2 new methods in ``TestManifestRecoveredFiles``:
- ``test_is_recovered_absolute_path_returns_false``
- ``test_is_recovered_escaping_path_returns_false``
🤖 AI disclosure: drafted with assistance from Claude (Opus 4.7).
* fix(manifest): clear recovered marker on managed re-record + reject '..' in is_recovered
Address Copilot Round-7 review comments on PR #2483:
1. record_existing(recovered=False) and record_file now BOTH discard the
path from _recovered_files. The marker is meant to flag "we observed
this file but cannot vouch it's a managed baseline" — once the same
path is re-recorded as managed (either explicitly or by writing fresh
bytes), the marker is stale and must clear so refresh_managed and
future is_recovered queries return the truthful answer.
2. is_recovered now applies the same canonical-key guard as record_existing
(rejects absolute paths and '..' segments lexically before delegating
to _validate_rel_path). Such paths can never be stored keys, so the
query correctly returns False without depending on _validate_rel_path
semantics that diverged from record_existing's stricter contract.
record_file docstring updated to mention the side-effect on recovered
markers.
Tests: 3 new methods in TestManifestRecoveredFiles covering
record_existing(false) clearing, record_file clearing, and is_recovered
dotdot rejection.
* test(manifest): update is_recovered comments to reflect Round-7 lexical guard
Round 8 — addresses Copilot review comment on tests/integrations/test_manifest.py:362.
After Round-7 (1dbf0c2), is_recovered() rejects absolute paths and '..' segments
up front via a lexical guard, returning False without calling _validate_rel_path
at all. The test comments still described the prior "_validate_rel_path raises;
we catch" code path, which is misleading for readers.
Updated comments in both:
- test_is_recovered_absolute_path_returns_false (Copilot's exact target)
- test_is_recovered_escaping_path_returns_false (same comment-class issue;
fixed preemptively to avoid a Round-9 finding on the same drift)
Pure documentation change. Test assertions and behavior unchanged; all manifest
tests still green.
* fix(manifest): document OS errors on record_existing + filter orphan recovered_files on load
Round 9 — addresses Copilot review on PR #2483:
1. record_existing's docstring now documents OSError/PermissionError as
possible raises (in addition to ValueError) — the implementation has
always been able to raise them from is_symlink, is_file, or the
file-read used to hash, but the contract did not reflect that.
Callers should be prepared for both surfaces.
2. load() now filters recovered_files entries that don't correspond to
keys in files. An externally-edited or partially-corrupted manifest
can deserialize with orphan recovered paths; rather than reject the
whole manifest (too strict on the upgrade path), we drop the orphans
and let the inconsistency self-correct on the next save(). is_recovered
then returns the truthful False for the orphan.
Tests: new test_load_filters_recovered_files_not_in_files asserting an
orphan recovered entry is dropped on load.
* chore: bump version to 0.9.1
* chore: begin 0.9.2.dev0 development
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* fix(cli): pin UTF-8 encoding on init-options and .extensionignore I/O
``Path.read_text`` / ``Path.write_text`` default to the system locale
codec, which is cp1252 / gb2312 / cp932 on Windows. Two user-facing
file paths in spec-kit were calling them without an explicit
``encoding=`` argument:
- ``src/specify_cli/__init__.py:400,412`` —
``save_init_options`` / ``load_init_options`` for
``.specify/init-options.json``. A peer machine with a different
default locale (or a UTF-8 Unix CI runner reading a file written on
a cp1252 Windows host) cannot decode the file, raising
``UnicodeDecodeError``. ``UnicodeDecodeError`` is a subclass of
``ValueError`` — not ``OSError`` / ``json.JSONDecodeError`` — so
the existing fall-back ``except`` tuple in ``load_init_options``
also misses it and the error propagates raw to the CLI.
- ``src/specify_cli/extensions.py:764`` — ``.extensionignore``
pattern reader. The very next line already normalises
backslashes "so Windows-authored files work", proving the codebase
expects Windows authors to write this file. Multibyte UTF-8
patterns (Chinese filenames, accented directory names) silently
mojibake when the host locale is not UTF-8, so the patterns fail
to match and unintended files are shipped with the extension.
The sibling integration-catalog reader at
``src/specify_cli/integrations/catalog.py:150,156,193,202,374``
already pins ``encoding="utf-8"`` everywhere. PR #2280 fixed the
symmetric PowerShell-template BOM bug. This change brings the two
remaining drifted paths in line with that precedent.
Regression tests:
- ``tests/test_presets.py::TestInitOptions`` — parametrized non-ASCII
round-trip (CJK, Latin-1, Greek, emoji) plus a corrupted-file case
that asserts the existing "fall back to {}" contract still holds
when a peer file contains bytes invalid as UTF-8.
- ``tests/test_extensions.py::TestExtensionIgnore`` — Japanese
(``ドキュメント/``) and Latin-1 (``café/``) ignore patterns
correctly exclude their directories during install.
* fix(cli): wrap .extensionignore decode error and tighten UTF-8 contract
Addresses Copilot review feedback on this PR.
Three issues, three fixes:
1. ``save_init_options`` now writes JSON with ``ensure_ascii=False``.
Without that flag, ``json.dumps`` emits ASCII-only ``\uXXXX``
escapes, which means the ``encoding="utf-8"`` pin on the
surrounding ``Path.write_text`` makes no observable difference for
any value we currently write. Flipping ``ensure_ascii`` makes the
non-ASCII bytes hit the file directly, so the encoding pin becomes
the thing that decides between cp1252 garbage and clean UTF-8 on
Windows. The comment above the call now describes the real reason
instead of the previously-misleading rationale Copilot flagged.
2. ``test_save_load_round_trip_preserves_non_ascii`` was a no-op under
the old ``ensure_ascii=True`` writer (Copilot's second comment).
Added ``test_save_writes_real_utf8_bytes`` that asserts the on-disk
bytes contain the UTF-8 encoding of ``café`` (``0xC3 0xA9``), not
its JSON escape form ``é``. Removing either
``ensure_ascii=False`` or ``encoding="utf-8"`` from the writer now
breaks this test — the contract is pinned.
3. ``.extensionignore`` reader wraps ``UnicodeDecodeError`` as
``ValidationError`` with a pointer to the offending byte
(Copilot's third comment). Mirrors
``ExtensionManifest._load_yaml``'s existing handler for
``extension.yml``. Adds
``test_extensionignore_invalid_utf8_raises_validation_error``
asserting installation aborts with the wrapped error instead of a
raw Python traceback.
The Hermes Agent integration ships in the CLI (src/specify_cli/integrations/hermes/)
and is registered in the catalog, but the supported-agents table in the
integrations reference omitted it. Add the row so the docs match the shipped
integration.
TestClineIntegration._expected_files() overrides the base-class version but
was not updated when the bundled agent-context extension files were added to
test_integration_base_markdown.py, causing test_complete_file_inventory_sh
and test_complete_file_inventory_ps to fail.
Fixes#2796
* Add spec-kit-linear extension to community catalog
Add linear extension submitted by @ashbrener to:\n- extensions/catalog.community.json\n- docs/community/extensions.md\n\nCloses #2778
* Address PR review feedback for spec-kit-linear entry
- Use Unicode arrow (→) in catalog/docs description\n- Move docs row to alphabetical Spec section
* Address follow-up review naming/order feedback
- Use human-friendly display name: Linear Integration\n- Move docs row to alphabetical L section
* chore: bump version to 0.9.0
* chore: begin 0.9.1.dev0 development
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* docs: add May 2026 newsletter
Publish the May 2026 newsletter documenting project milestones including:
- Crossing 100K GitHub stars and top-100 GitHub project status
- 100+ community extensions in catalog
- Fourteen releases (v0.8.4–v0.8.17)
- Multi-agent install support and constitution governance features
- Open Source Friday livestream and media coverage across 25+ languages
- Industry analyst recognition
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix: move URL install confirmation prompt before spinner (#2783)
The typer.confirm() prompt inside console.status() was overwritten by
Rich's spinner animation, making extension add --from <url> appear hung.
Move URL validation and the default-deny confirmation prompt before the
spinner block so the user can see and respond to the [y/N] prompt.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix: guard prompt with not dev, escape from_url in Rich markup
Address PR review feedback:
- Gate URL confirmation prompt on 'not dev' so --dev + --from does not
show a confusing prompt for a URL path that will be ignored.
- Escape from_url with rich.markup.escape() in both the warning panel
and the download message to prevent markup injection via crafted URLs.
* fix: remove unused import, reuse safe_url, add regression tests
Address second round of PR review:
- Remove unused urllib.request import from URL install path
- Remove redundant re-import of rich.markup.escape; reuse safe_url
computed before the spinner for download and error messages
- Add test_add_from_url_prompts_before_spinner: asserts typer.confirm
fires before console.status spinner to prevent #2783 regression
- Add test_add_from_url_cancel_exits_cleanly: asserts declining the
prompt exits with code 0 and prints Cancelled
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Initial plan
* Extract agent context updates into bundled agent-context extension
* Potential fix for pull request finding 'Unused import'
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
* Potential fix for pull request finding 'Unused import'
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
* fix: address review comments on agent-context extension
- bash: parse init-options.json with a single python3 invocation instead
of three separate read_json_field calls, for parity with the PowerShell
ConvertFrom-Json approach and to avoid divergent error semantics
- bash: use parameter expansion to strip PROJECT_ROOT prefix from plan
path instead of sed interpolation, avoiding special-character fragility
- powershell: limit Get-ChildItem to -Depth 1 so plan.md discovery matches
the bash glob specs/*/plan.md (one level deep) — fixes cross-platform
inconsistency with nested plan.md files
- powershell: replace Substring+Length relative-path with
[System.IO.Path]::GetRelativePath for robustness across case/PSDrive
differences
- __init__.py: move agent-context extension install to after
save_init_options so init-options.json is present when hooks run
- __init__.py: seed context_markers in init-options only when
context_file is truthy; avoids noise for integrations without a context
file
- integrations/base.py: narrow blanket except Exception in
_resolve_context_markers to ImportError / (OSError, ValueError) so
unexpected bugs surface instead of being silently swallowed
* fix: gate context_markers in _update_init_options_for_integration on context_file
Apply the same gating logic used during `specify init`: only write
context_markers to init-options.json when the integration actually has a
context_file set. When switching to an integration without a context file
the stale markers are removed, keeping the two init paths consistent.
* fix: move context_file/context_markers from init-options.json to agent-context extension config
* Potential fix for pull request finding 'Unused global variable'
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
* fix: clarify local import comment in agents.py
* Fix remaining agent-context review findings
* Fix follow-up agent-context review issues
* Address review feedback: narrow except, improve PyYAML messaging, surface config-written note
* Fix double-space in PyYAML install hint message
* Potential fix for pull request finding 'Empty except'
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
* Potential fix for pull request finding 'Empty except'
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
* Address latest agent-context review feedback
* Harden bash config parse output handling
* Clarify ImportError-only fallback comment
* Apply review feedback: drop dead try/except, guard ext-config creation, explicit ConvertFrom-Yaml check
* Remove redundant $Options = $null in PS1 catch block
* Add constitution directives, deprecation warning, agent-context auto-install, and init flow fix
- Add constitution-loading directive to specify, clarify, tasks, checklist, taskstoissues commands
- Add deprecation warning (v0.12.0) in upsert_context_section()
- Auto-install agent-context extension during specify init
- Move context_file from init-options.json to agent-context extension config
- Add tests: deprecation warning, corrupt config, constitution directives
- Update file inventories across all integration tests
* Address review: fix init ordering, test coverage, and hermes inventory
- Move agent-context extension install after init-options.json is saved
so skill registration can read ai_skills + integration key
- Write extension config after install (avoids template overwriting context_file)
- Fix test_defaults_when_markers_field_missing to truly test missing markers key
- Update hermes tests to allow extension-installed agent-context skill
* Address review: chmod ordering, preserve markers, PS1 Python check, YAML key order
- Move ensure_executable_scripts after agent-context extension install
so extension scripts get execute bits set
- Use preserve_markers=True on reinit to keep user-customized markers
- Add Python 3 version check in PowerShell fallback (matching bash behavior)
- Add sort_keys=False to yaml.safe_dump for stable config output
* Address review: path traversal guards and docstring fix
- Reject absolute paths and '..' segments in context_file in both bash and
PowerShell scripts to prevent writes outside the project root
- Fix docstring in _update_init_options_for_integration to accurately
describe marker preservation behavior
* Address review: strict enabled check, docstring, segment-level path traversal
- Use 'is not False' for enabled check so only literal False disables
- Update upsert_context_section docstring to mention disabled-extension return
- Fix path traversal guards to check actual path segments, not substrings
(allows filenames like 'notes..md' while rejecting '../' traversal)
* Address review: UnicodeError handling, missing extension warning
- Add UnicodeError to exception tuples in _load_agent_context_config and
_resolve_context_markers so garbled UTF-8 config files fall back to defaults
- Emit error (with reinstall command) instead of silent skip when bundled
agent-context extension is not found during init
* Address review: bash backslash traversal guard, wheel packaging
- Reject backslash separators and Windows drive-letter paths in bash
context_file validation (prevents traversal on Git-Bash/Windows)
- Add extensions/agent-context to pyproject.toml force-include so the
bundled extension is included in wheel builds
* Address review: write extension config before init-options.json
- Reorder writes in _update_init_options_for_integration so the
agent-context extension config is updated first; if it fails,
init-options.json remains consistent with the previous state
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
* chore: bump version to 0.8.18
* chore: begin 0.8.19.dev0 development
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* Initial plan
* feat: support SPECKIT_INTEGRATION_<KEY>_EXECUTABLE env var override
Adds `IntegrationBase._resolve_executable()` which reads
`SPECKIT_INTEGRATION_<KEY>_EXECUTABLE` (hyphens→underscores, uppercased)
and falls back to `self.key` when unset or whitespace-only.
All concrete `build_exec_args()` implementations now call
`self._resolve_executable()` instead of hard-coding `self.key` or
`"agy"` as the first argv token:
- MarkdownIntegration, TomlIntegration, SkillsIntegration (base classes)
- CodexIntegration, DevinIntegration, OpencodeIntegration, HermesIntegration, AgyIntegration
- CopilotIntegration (overrides `_resolve_executable()` to fall back to
the platform-specific `_copilot_executable()` default; also updates
`dispatch_command()` to use `_resolve_executable()`)
Tests added to tests/integrations/test_extra_args.py covering:
- default (unset) falls back to key
- env var replaces first argv token
- whitespace-only env var is a no-op
- key hyphen→underscore normalisation
- cross-integration scoping (wrong key ignored)
- all override integrations (Codex, Devin, Opencode, Copilot)
- Copilot dispatch_command path
- EXECUTABLE and EXTRA_ARGS can be set simultaneously
See issue #2596."
* fix: complete docstring sentence in _resolve_executable
* test: generalize extra-args test comments for override coverage
* Fix stale Codex executable comment
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Add noop: report-as-issue: false to safe-outputs frontmatter in both
add-community-extension and add-community-preset workflows to prevent
them from posting noise comments to the [aw] No-Op Runs tracking issue.
Closes#2747
Display a yellow warning panel and default-deny [y/N] prompt when
installing extensions via --from <url>, since this bypasses the
catalog trust boundary.
Both add-community-preset and add-community-extension workflows previously
triggered on issues opened, edited, and labeled events. This caused them to
fire on every new issue and post noisy bot comments explaining the issue
wasn't a submission (see #2739).
Changes:
- Narrow trigger from [opened, edited, labeled] to [labeled] only
- Update prompt instructions to stop silently on non-matching issues
instead of posting a comment
* feat(integrations): support SPECIFY_<KEY>_EXTRA_ARGS env var for agent subprocess flags
Read a per-integration env var (SPECIFY_<KEY>_EXTRA_ARGS) inside
`SkillsIntegration.build_exec_args`, `MarkdownIntegration.build_exec_args`,
and `TomlIntegration.build_exec_args` and append the parsed flags to the
spawned agent's argv, gated per integration key.
Operators can now opt into extra CLI flags (e.g.
`SPECIFY_CLAUDE_EXTRA_ARGS=--dangerously-skip-permissions`) without
modifying any SKILL or workflow YAML. Useful in CI / non-interactive
contexts where the spawned `<agent> -p ...` would otherwise hang on
an internal permission or input prompt invisible to the parent
`specify workflow run` process.
Key normalization: `kiro-cli` → `SPECIFY_KIRO_CLI_EXTRA_ARGS` (hyphen
replaced with underscore, then uppercased).
Default (env var unset or whitespace-only) is byte-identical to
previous behaviour. Extra args are inserted between `-p prompt` and
the model / output-format flags so they cannot clobber canonical
Spec Kit args.
Implementation: a single helper `IntegrationBase._apply_extra_args_env_var`
encapsulates the env-var read + shlex parsing; each of the three
concrete `build_exec_args` implementations calls it.
Closes#2595
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(integrations): wire SPECIFY_<KEY>_EXTRA_ARGS into Codex/Devin/Opencode/Copilot
Four integrations override `build_exec_args` and were silently
ignoring the env-var hook introduced in the previous commit:
- CodexIntegration (`codex exec ...`)
- DevinIntegration (`devin -p ...`)
- OpencodeIntegration (`opencode run ...`)
- CopilotIntegration (`copilot -p ...`)
Each now calls `self._apply_extra_args_env_var(args)` between the
base argv and the canonical Spec Kit flags (matching the placement
in `MarkdownIntegration`, `TomlIntegration`, and `SkillsIntegration`),
so operator-injected flags cannot clobber `--model` / `--output-format`
/ `--json`.
Adds 4 parameterized override-integration tests locking the wiring
per agent. Also cleans up an inline `__import__("os").environ` in the
fixture to a top-of-file `import os`.
Drive-by typing fix: guard `self.registrar_config.get(...)` in
`CopilotIntegration.add_commands` against the `None` case, matching
the pattern already used in `base.py` for the same access.
Addresses Copilot review on #2596.
* fix(integrations): apply Opencode extra-args before prompt-derived --command
When the Opencode prompt starts with `/`, `build_exec_args` injects
`--command <X>` derived from the prompt. The previous placement of
`self._apply_extra_args_env_var(args)` appended operator-injected
args AFTER that `--command`, so a user setting
`SPECIFY_OPENCODE_EXTRA_ARGS="--command override"` could redirect the
command under typical last-wins repeated-flag CLI semantics.
Move the hook to immediately after `args = [self.key, "run"]`, before
the prompt-parsing block. The operator's `--command override` (if
any) now precedes the Spec Kit-derived `--command speckit`, so the
canonical choice wins.
Adds `test_opencode_extra_args_cannot_clobber_prompt_derived_command`
locking the ordering. Also corrects the module docstring to describe
the hook as living in `IntegrationBase` (not `SkillsIntegration`) and
to acknowledge that this file covers Codex/Devin/Opencode/Copilot in
addition to SkillsIntegration stubs.
Addresses Copilot review on #2596.
* fix(integrations): honour SPECIFY_COPILOT_EXTRA_ARGS in dispatch_command
`CopilotIntegration` is the only integration that overrides
`dispatch_command` — it builds `cli_args` inline rather than going
through `build_exec_args`. The previous commit wired
`_apply_extra_args_env_var` into `build_exec_args` but workflow
execution calls `dispatch_command`, so `SPECIFY_COPILOT_EXTRA_ARGS`
was silently ignored at runtime.
Add the hook in `dispatch_command` immediately after
`cli_args = ["copilot", "-p", prompt]`, mirroring the placement in
`build_exec_args` (between `-p prompt` and the canonical
`--agent` / `--yolo` / `--model` / `--output-format` flags).
`IntegrationBase.dispatch_command` already delegates to
`build_exec_args`, so Codex, Devin, and Opencode continue to honour
their respective env vars through inheritance — no further changes
needed for them.
Adds two end-to-end tests that monkeypatch `subprocess.run` and
assert the env-var args reach the executed argv:
- `test_copilot_dispatch_command_includes_extra_args` locks the
bypass fix on the overridden path.
- `test_codex_dispatch_command_includes_extra_args` locks the
inherited-base-dispatch path for the other three integrations.
Addresses Copilot review on #2596.
* refactor(integrations): rename env var to SPECIFY_INTEGRATION_<KEY>_EXTRA_ARGS
Per maintainer request: scope the operator-injected env var to the
integration subsystem by prepending `INTEGRATION_` to the key
segment, so it does not collide with other Spec Kit env-var
namespaces.
Renames everywhere it appears:
- Helper `IntegrationBase._apply_extra_args_env_var` env_name
format and docstring (`base.py`).
- Inline comment in `CopilotIntegration.dispatch_command`.
- All `monkeypatch.setenv(...)` calls, docstrings, and the
autouse fixture's scope filter in
`tests/integrations/test_extra_args.py`.
No behaviour change beyond the variable name. Default (env var
unset) still byte-identical to before this PR.
Addresses review on #2596.
* fix(integrations): raise actionable error on malformed EXTRA_ARGS quoting
Wrap `shlex.split` in `_apply_extra_args_env_var` so an unmatched quote
in `SPECIFY_INTEGRATION_<KEY>_EXTRA_ARGS` surfaces a clear `ValueError`
naming the offending env var and showing the invalid value, instead of
crashing workflow dispatch with a bare shlex traceback. Adds a new test
locking the actionable error path.
Addresses Copilot review feedback on #2596.
* test(integrations): use `_copilot_executable()` in Copilot extra-args test
`test_copilot_integration_honours_extra_args` hardcoded `"copilot"`
in the expected argv, but `CopilotIntegration.build_exec_args` calls
`_copilot_executable()` which returns `"copilot.cmd"` on Windows
(`os.name == "nt"`). The test passed on Linux/macOS and failed on
all three Windows-latest matrix entries.
Resolve by importing `_copilot_executable` alongside `CopilotIntegration`
and using it as the first expected argv token. The companion
`test_copilot_dispatch_command_includes_extra_args` already uses
`index()` lookups rather than full-argv equality so it was unaffected.
* fix(integrations): couple Codex executable to self.key + cover base classes
Two Copilot findings on the latest pass:
1. `CodexIntegration.build_exec_args` hardcoded the executable name
as the literal `"codex"` while the env-var lookup derives from
`self.key`. The two should stay coupled (matching Devin/Opencode,
which both use `self.key` already). Replace the literal with
`self.key` so the argv and env-var scoping cannot drift.
2. `tests/integrations/test_extra_args.py` covered the
`SkillsIntegration` mechanism (via stubs near the top) and the
four `build_exec_args` overrides (Codex/Devin/Opencode/Copilot)
end-to-end, but did not exercise the `MarkdownIntegration` or
`TomlIntegration` base implementations directly. Add bare
`_MarkdownAgentStub` and `_TomlAgentStub` test stubs and a test
each — the most common integration pattern (Amp, Auggie, Generic,
Gemini, Tabnine, …) inherits without overriding, so the base
wiring is now locked.
Full suite: 3011 passed (was 3009), 40 skipped, no regressions.
Addresses Copilot review feedback on #2596.
* fix(integrations): rename env var to SPECKIT_INTEGRATION_<KEY>_EXTRA_ARGS
Renames the env-var hook prefix from `SPECIFY_INTEGRATION_*` to
`SPECKIT_INTEGRATION_*` to match the established codebase
convention for integration-subsystem env vars
(`SPECKIT_INTEGRATION_CATALOG_URL` in `integrations/catalog.py`,
`SPECKIT_COPILOT_ALLOW_ALL_TOOLS` in `integrations/copilot/__init__.py`).
The `SPECIFY_*` prefix is reserved for user-facing
feature-resolution variables (`SPECIFY_FEATURE`,
`SPECIFY_FEATURE_DIRECTORY`); reusing it for integration-subsystem
scoping would introduce a second integration namespace under a
different prefix, confusing operators who already set
`SPECKIT_INTEGRATION_CATALOG_URL`.
Also reverts the unrelated defensive `arg_placeholder` /
`registrar_config is None` guard in
`CopilotIntegration.setup_skills_mode` — it was a drive-by pyright
cleanup mixed into this PR. Every concrete integration sets
`registrar_config` so the guard never fires in practice; the
typing issue belongs in a focused follow-up rather than this
env-var-hook PR.
Updates everywhere the old prefix appeared:
- `IntegrationBase._apply_extra_args_env_var` helper + docstring
- `CopilotIntegration.dispatch_command` inline comment
- All `monkeypatch.setenv(...)` calls in `tests/integrations/test_extra_args.py`
- The autouse fixture scope filter
- Test module docstring
Full suite: 3011 passed, 40 skipped, no regressions.
Addresses Copilot review feedback on #2596.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: bump version to 0.8.17
* chore: begin 0.8.18.dev0 development
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* docs: consolidate Community sections in README
Replace four separate Community sections (Extensions, Presets,
Walkthroughs, Friends) with a single consolidated section containing
a bullet list, one shared disclaimer, and both publishing guide links.
* fix: broken community anchor links and missing Hermes hook note injection
- Update README.md and extensions/README.md to point community
extension links to the docs site instead of removed section anchor
- Add post_process_skill_content() call in Hermes setup() so hook
command notes are injected into generated skills
- Add Hermes test override for test_hook_sections_explain_dotted_command_conversion
with Path.home() monkeypatch
* feat(agy): enhance Google Antigravity CLI integration
- Set requires_cli=True and install_url for CLI tool detection
- Implement build_exec_args() for non-interactive execution via agy --print
- Add dot-to-hyphen hook command note injection in generated SKILL.md files
* fix(agy): add --ignore-agent-tools to TestAgyAutoPromote tests
Tests verify file layout and setup warnings, not CLI presence.
agy requires_cli=True causes CI failures when agy is not installed.
