Add John Lam as contributor and release badge by localden · Pull Request #1 · github/spec-kit

localden · 2025-08-25T18:26:40Z

Adds John Lam (@jflam) to the maintainers section
Includes GitHub Actions release workflow badge in the header
Updates README formatting for better visibility
Fixes release workflow to work with repository rules by removing problematic direct push to main branch

@jflam

- Add John Lam (@jflam) to maintainers section - Include GitHub Actions release workflow badge in header - Update README formatting for better visibility

Copilot

Pull Request Overview

This PR adds John Lam as a maintainer and enhances the README with a GitHub Actions release workflow badge for better project visibility and transparency.

Adds John Lam (@jflam) to the maintainers section
Includes a GitHub Actions release workflow badge in the header

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Add AGENTS.md contextual file

Update template path for spec file creation

Add John Lam as contributor and release badge

Update template path for spec file creation

Implements comprehensive git worktree integration enabling developers to work on specifications in the main repo while simultaneously implementing features in isolated worktrees. Supports automatic creation, manual creation, listing, removal, and cleanup of worktrees. Features: - Automatic worktree creation during /speckit.specify workflow - Manual worktree creation via /speckit.worktree command - List all worktrees with status and disk usage (/speckit.worktree list) - Safe removal with uncommitted change warnings (/speckit.worktree remove) - Batch cleanup of stale worktrees (/speckit.worktree cleanup) - Cross-platform support (bash + PowerShell) - Conflict resolution prompts (stop/cleanup/skip) - Branch and specs directory protection User Stories Implemented: - US1 (P1): Automatic worktree creation - MVP functionality - US2 (P2): Manual worktree creation for existing branches - US3 (P2): View worktree status with table display - US4 (P3): Remove specific worktree with safety checks - US5 (P3): Clean up all stale worktrees in batch Technical Implementation: - scripts/bash/manage-worktrees.sh - Core bash functions - scripts/powershell/manage-worktrees.ps1 - PowerShell equivalent - Integration with create-new-feature scripts - .claude/commands/speckit.worktree.md - Claude Code command - templates/commands/worktree.md - Generic AI agent template - Updated documentation (CLAUDE.md, README.md) This enables parallel development: main repo for specs/planning, worktrees for implementation. Both work simultaneously without conflicts. Fixes github#1-git-worktree-support 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

[ImgBot] Optimize images

…r feedback) Implements fixes for workflow-reviewer agent's identified critical issues: 1. Fix State Management Gap (Critical github#1) - Added spec-metadata.json generation in /speckit.quick Phase 1 - Enables integration with /speckit.status and /speckit.pm (v2.3 compatibility) - Metadata tracks: workflow_type, phase, approvals, risk_level - Updates metadata after each phase (pre-flight, implementation, quality gate, complete) - File: .specify/quick-tasks/quick-task-[timestamp]-metadata.json 2. Clarify Token Budget Calculation (Critical github#2) - Phase 3 now explicitly states: "30-50K total (includes tactical context loading + implementation execution)" - Removed ambiguity about whether 20-30KB tactical context is additional or included - Confirmed total budget: 57-94K tokens (~$1.10-$1.80) 3. Verify Documentation Consistency (Critical github#3) - Verified command counts are correct: 18 core + 3 epic = 21 total - Confirmed /speckit.quick is in all relevant tables (CLAUDE.md, README.md) - No changes needed - documentation was already accurate 4. Add Risk Scoring to Step 0.5 (Major github#4) - Added heuristic risk assessment BEFORE complexity analysis - HIGH-RISK indicators: payment, auth, multi-tenant, compliance (GDPR/HIPAA/PCI), database migration - MEDIUM-RISK indicators: database, schema change, API endpoint, real-time, bulk operations - Decision logic: - ANY HIGH-RISK keyword → Block quick workflow, require full workflow - ≥2 MEDIUM-RISK keywords → Block quick workflow, recommend full workflow - ELSE → LOW-RISK (0-3) → Continue to complexity analysis - Prevents users from accidentally using /speckit.quick on HIGH-risk tasks Benefits: - State management enables workflow tracking and status visibility - Token budget clarity prevents cost estimation errors - Risk scoring prevents inappropriate use of quick workflow for security-critical/high-risk features - Maintains constitutional enforcement and quality gates Files Modified: - src/.claude/commands/speckit.quick.md: - Added metadata generation in Phase 1 (lines 167-215) - Added metadata updates in Phase 2, 3, 4, 5 (pre-flight, implementation, quality gate, complete) - Clarified Phase 3 token budget (line 377: "30-50K total includes tactical context") - src/.claude/commands/speckit.specify.md: - Added Quick Risk Assessment to Step 0.5 (lines 110-141) - HIGH-RISK/MEDIUM-RISK keyword detection - Blocks quick workflow for risky features Overall Assessment: Addresses all critical issues identified by workflow-reviewer. Estimated improvement: 8.5/10 (was 7.2/10) Version: v2.9.1 (patch)

…ring-issue Fix branch numbering to increment globally across all features

…heckpointing, Vision) pull request github#1 from panaversity/ai_assistant_mastery

The validation script expects all command files to have an 'agent: pmfkit.*' field in their YAML frontmatter, but the generate_commands function was not adding this field. This caused validation failures during release workflow. Changes: - Modified generate_commands to inject 'agent: pmfkit.{name}' after opening frontmatter delimiter - Maintains correct YAML structure - Ensures all generated commands pass validation check github#1 Fixes GitHub Actions workflow failure where validation reported: "No 'agent:' frontmatter found in pmfkit.specify.md" 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>

This commit adds a complete verification-driven approach to code analysis, addressing the critical issue: how do we know AI analysis results are correct? Key innovation: Analysis Verification Loop - AI analyzes code → generates hypotheses → creates verification tests → runs tests → verifies hypotheses → outputs only verified findings Core additions: 1. Comprehensive design doc: docs/code-review-with-verification.md - Explains why verification is essential - 6-step verification workflow (hypothesis → test → verify → report) - Solves AI hallucination through executable tests - Inspired by spec-kit's verification-driven approach 2. Complete spec example: examples/verified-code-review.spec.yaml - Detailed workflow with verification steps - Behavioral tests (verify code does what it should) - Exploit tests (verify security vulnerabilities are real) - Benchmark tests (verify performance issues exist) - GitHub Issue-style output format 3. Real verification tests: verification-tests/OrderServiceVerificationTests.java - 7 complete JUnit tests validating business logic - Tests for transaction consistency, concurrency, data flow - Self-documenting: tests that fail = bugs found - Includes expected failures (H2, H5) proving bugs exist 4. Example output: 04-verified-business-logic-issues.md - GitHub Issue-style report - Issue github#1: Transaction consistency bug (with test proof) - Issue github#2: Concurrent overselling bug (with test proof) - Each issue includes: code evidence, test code, test output, impact analysis, reproduction steps, and fix suggestions - Only outputs verified findings (no speculation) Why this matters: - Traditional AI analysis: AI says "there might be a bug" → user must verify manually - Verification-driven: AI says "bug confirmed by test" → user sees executable proof - Reduces false positives, increases confidence - Makes code review results actionable and trustworthy Alignment with spec-kit philosophy: - spec-kit: write code → run tests → verify → iterate - code-review: analyze code → generate tests → verify → report - Both use verification as the source of truth

…mplates (#1702) * fix: wire after_tasks and after_implement hook events into command templates (#1701) The HookExecutor backend in extensions.py was fully implemented but check_hooks_for_event() was never called by anything — the core command templates had no instructions to check .specify/extensions.yml. Add a final step to templates/commands/tasks.md (step 6, after_tasks) and templates/commands/implement.md (step 10, after_implement) that instructs the AI agent to: - Read .specify/extensions.yml if it exists - Filter hooks.{event} to enabled: true entries - Evaluate any condition fields and skip non-matching hooks - Output the RFC-specified hook message format, including EXECUTE_COMMAND: markers for mandatory (optional: false) hooks Bumps version to 0.1.7. * fix: clarify hook condition handling and add YAML error guidance in templates - Replace ambiguous "evaluate any condition value" instruction with explicit guidance to skip hooks with non-empty conditions, deferring evaluation to HookExecutor - Add instruction to skip hook checking silently if extensions.yml cannot be parsed or is invalid * Fix/extension hooks not triggered (#1) * feat(templates): implement before-hooks check as pre-execution phase * test(hooks): create scenario for LLMs/Agents on hooks --------- Co-authored-by: Dhilip <s.dhilipkumar@gmail.com>

* Rewrite AGENTS.md for integration subpackage architecture Replaces the old AGENT_CONFIG dict-based 7-step process with documentation reflecting the integration subpackage architecture shipped in #1924. Removed: Supported Agents table, old step-by-step guide referencing AGENT_CONFIG/release scripts/case statements, Agent Categories lists, Directory Conventions section, Important Design Decisions section. Kept: About Spec Kit and Specify, Command File Formats, Argument Patterns, Devcontainer section. Added: Architecture overview, decision tree for base class selection, configure/register/scripts/test/override steps with real code examples from existing integrations (Windsurf, Gemini, Codex, Copilot). Agent-Logs-Url: https://github.com/github/spec-kit/sessions/71b25c53-7d0c-492a-9503-f40a437d5ece Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * Fix JSONC comment syntax in devcontainer example Agent-Logs-Url: https://github.com/github/spec-kit/sessions/71b25c53-7d0c-492a-9503-f40a437d5ece Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * docs(AGENTS.md): address Copilot PR review comments - Clarify that integrations are registered by _register_builtins() in __init__.py, not self-registered at import time - Scope the key-must-match-executable rule to CLI-based integrations (requires_cli: True); IDE-based integrations use canonical identifiers - Replace <commands_dir> placeholder in test snippet with a concrete example path (.windsurf/workflows/) - Document that hyphens in keys become underscores in test filenames (e.g. cursor-agent -> test_integration_cursor_agent.py) - Note that the argument placeholder is integration-specific (registrar_config["args"]); add Forge's {{parameters}} as an example - Apply consistency fixes to Required fields table, Key design rule callout, and Common Pitfalls #1 * docs(AGENTS.md): clarify scripts path uses Python-safe package_dir not key The scripts step previously referenced src/specify_cli/integrations/<key>/scripts/ but for hyphenated keys the actual directory is underscored (e.g. kiro-cli -> kiro_cli/). Rename the placeholder to <package_dir> and add a note explaining: - <package_dir> matches <key> for non-hyphenated keys - <package_dir> uses underscores for hyphenated keys (e.g. kiro-cli -> kiro_cli/) - IntegrationBase.key always retains the original hyphenated value Addresses: #2119 (comment) * docs(AGENTS.md): use <key_with_underscores> in pytest example command The pytest command previously used <key> as a placeholder, but test filenames always use underscores even for hyphenated keys. This was internally inconsistent since the preceding sentence already explained the hyphen→underscore mapping. Switch to <key_with_underscores> to match the actual filename on disk. Addresses: #2119 (comment) * docs(AGENTS.md): use <package_dir> in step 2 subpackage path The path src/specify_cli/integrations/<key>/__init__.py was inaccurate for hyphenated keys (e.g. kiro-cli lives in kiro_cli/, not kiro-cli/). Rename the placeholder to <package_dir>, define it inline (hyphens become underscores), and note that IntegrationBase.key always retains the original hyphenated value. Addresses: #2119 (comment) * docs(AGENTS.md): qualify 'single source of truth' to Python metadata only The registry is only authoritative for Python integration metadata. Context-update dispatcher scripts (bash + PowerShell) still require explicit per-agent cases and maintain their own supported-agent lists until they are migrated to registry-based dispatch. Tighten the claim to avoid misleading contributors into skipping the script updates. Addresses: #2119 (review) * docs(AGENTS.md): mention ValidateSet update in PowerShell dispatcher step The update-agent-context.ps1 script has a [ValidateSet(...)] on the AgentType parameter. Without adding the new key to that list, the script rejects the argument before reaching Update-SpecificAgent. Add this as an explicit step alongside the switch case and Update-AllExistingAgents. Addresses: #2119 (review) * fix(integrations): sort codebuddy before codex in _register_builtins() Both the import list and the _register() call list had codex before codebuddy, violating the alphabetical ordering that AGENTS.md documents. Swap them so the file matches the documented convention. Addresses: #2119 (review) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>

…ck orphans, backup-read boundaries, and Rich escaping 1. WorkflowRegistry._load(): a symlinked .specify/.specify/workflows parent (or a symlinked registry file) silently returned an empty registry instead of raising, unlike an unreadable-file read failure. A read-only caller (notably the bundler's remove path) querying is_installed() before ever writing could conclude an installed workflow is absent, skip removing it, then delete the bundle record -- leaving the workflow untracked but still on disk. Now raises OSError immediately, matching the existing unreadable-file fail-closed behavior. 2/8. _validate_and_install_local and _install_workflow_from_catalog: when the destination directory already existed but had no prior workflow.yml (e.g. a leftover empty dir), existed_before was True but there were no backup bytes to restore, so the rollback closure did nothing on a later failure -- leaving the newly copied/ downloaded file behind. Both now unlink the newly created file in this case, restoring the pre-existing directory to its prior (empty) state. 3/4. Both install paths read the prior workflow.yml bytes (to seed the reinstall rollback) *before* any try/except boundary: a read failure on the existing file (e.g. a transient permission/FS issue) leaked a raw, unescaped OSError instead of the same clean CLI error used by every other failure branch in these functions. Both reads are now guarded by their own try/except OSError, with no writes attempted before the read succeeds (so there is nothing to roll back on this specific failure). 5. remove_bundle's exception-conversion message unconditionally claimed "No changes were recorded," even though a failure can occur after earlier components in the same bundle have already been removed from disk (save_records never runs on this path, so the record is left claiming the bundle fully installed). The message now reports how many components were already removed when that happened, instead of asserting no changes occurred. 6/7. workflow_remove's new post-registry-removal directory-failure error and its restore-failure warning interpolated workflow_dir and the exception values into Rich markup unescaped. A project path or OS error message containing Rich-markup-like brackets could be parsed as markup and hide/corrupt the displayed text. Both now use the existing _escape_markup helper, consistent with every other error path in this file. Tests (tests/test_workflows.py unless noted): - TestWorkflowRegistry::test_load_symlinked_workflows_dir_fails_closed_not_silently_empty (1) - TestWorkflowCliAlignment::test_add_dev_fresh_install_into_preexisting_empty_dir_cleans_new_file (2) - TestWorkflowCliAlignment::test_add_catalog_fresh_install_into_preexisting_empty_dir_cleans_new_file (8) - TestWorkflowCliAlignment::test_add_dev_reinstall_backup_read_failure_gives_clean_error (3) - TestWorkflowCliAlignment::test_add_catalog_reinstall_backup_read_failure_gives_clean_error (4) - tests/integration/test_bundler_install_flow.py::test_remove_partial_failure_message_reflects_partial_state (5) - TestWorkflowRemoveGuard::test_remove_directory_and_restore_failure_escapes_rich_markup (6/7) All seven were confirmed red beforehand, matching each thread's described failure mode exactly (silent empty registry instead of a raise; orphaned new file left behind; raw unescaped OSError leaking; a misleading "no changes were recorded" claim; Rich markup consuming bracketed path/exception text). Also updated test_registry_save_refuses_symlinked_parent, a pre-existing test that asserted the symlinked-parent raise at add()/save() time -- it now raises at construction instead, per fix github#1, so the test was adjusted to match without weakening its guarantee (still asserts no writes occur under the symlinked target). Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…3419) * feat(workflows): align workflow CLI with extension command surface Adds the missing workflow commands and flags so the workflow CLI matches the extension/preset pattern: add --dev and --from, search --author, update, enable and disable. Disabled workflows are blocked from running and marked in list output. Fixes #2342 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): preserve disabled state on update, guard corrupted registry entries Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): guard list against corrupted registry entries, re-raise typer.Exit in catalog install workflow list now skips non-dict registry entries with a warning instead of crashing, matching update/enable/disable. The broad except in _install_workflow_from_catalog no longer swallows typer.Exit, so precise errors like the non-HTTPS redirect message are not duplicated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): escape rich markup in id-mismatch errors and validate --from source early The two id-mismatch error paths interpolated repr() into Rich markup, so a stray bracket in a user typo could be parsed as markup. Route both through rich.markup.escape. `workflow add <source> --from <url>` also validated the source only after downloading. Validate it up front so a URL/path/typo fails without a network fetch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): escape rich markup in list output and catalog install errors, isolate update failures workflow list now escapes id/name/version/description before printing, matching how extensions render user-editable fields. The catalog install helper computes safe_wf_id once and uses it for every early error path plus the final failure message. workflow update wraps _safe_workflow_id_dir and the backup read inside the try/except typer.Exit block so an unsafe id in a corrupted registry fails that one workflow and the rest continue. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): escape rich markup in --from download exception message Matches how the catalog install path escapes exception strings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): catch OSError in per-workflow update loop and make restore best-effort Transient FS errors (perms, disk full) from backup read or write no longer abort the whole update run. The restore is wrapped in its own try/except so a failed write only warns, and the offending workflow is reported via 'Failed to update' like other per-workflow failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): escape rich markup in search output workflow search now escapes catalog-derived name/id/version/description/ tags before printing, matching extension search and workflow list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: escape workflow validation errors before Rich output Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): escape remaining unescaped Rich markup paths Covers the last few review threads not yet addressed: - Escape yaml.YAMLError text in the local workflow add install path (matches the already-escaped download/catalog paths). - Escape the non---dev local directory fallback's "No workflow.yml found in <path>" message (the --dev branch already escaped it). - Escape the redirected final_url in the --from non-HTTPS redirect error (IPv6 literals like http://[::1]/... are legal and contain brackets). - Escape the "Downloaded workflow is invalid" exception message in _install_workflow_from_catalog, matching the sibling catalog-install exception handler a few lines above it. Adds regression tests for each in TestWorkflowCliAlignment, following the existing escaping-test pattern in this class. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): escape workflow name/id in install success messages Workflow names and ids come from user-controlled YAML or external catalog data; printing them unescaped lets bracket characters be interpreted as Rich tags. Escape them in the add/catalog-install success messages and the remaining catalog error paths, matching the rest of the output hardening. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): fail cleanly on unparseable catalog install URLs urlparse raises ValueError on e.g. an unbalanced IPv6 literal before the invalid-URL branch is reached; on workflow update that also bypassed the per-workflow handler and aborted the whole command. Convert the parse failure into a clean error so add fails cleanly and update skips just the affected workflow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): reject catalog updates whose downloaded version mismatches The update path never verified the downloaded workflow carries the catalog version that triggered the update, so a stale or misconfigured URL could report success while leaving the old version installed or downgrading it. Pass the expected version into the install helper and fail the update when the downloaded definition does not match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): validate workflow ID in run command and document new CLI flags Path-equivalent spellings like "align-wf/" previously bypassed the registry disabled check because the engine normalizes the path while the registry matches the raw string. workflow run now validates non-file sources against the workflow ID pattern before lookup. Also updates docs/reference/workflows.md with --dev/--from install options, update/enable/disable commands, and the search --author flag. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): enforce disabled state for direct paths to installed workflows Running the installed copy's YAML directly (specify workflow run .specify/workflows/align-wf/workflow.yml) skipped the registry check. File sources resolving inside .specify/workflows/<id>/ now map back to the workflow ID and refuse to run while disabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): reject explicit empty --from URL instead of catalog fallback 'workflow add foo --from ""' fell through 'from_url or ...' to a catalog install. Distinguish None from empty string so explicit values stay on the URL-validation path and fail closed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): registry rollback on save failure, consistent disabled check, honest update summary - WorkflowRegistry.add now rolls back its in-memory mutation when save() raises, so a later successful save cannot persist metadata for a failed update alongside the restored YAML backup. - workflow run uses the same truthiness check for 'enabled' as list and disable, so malformed values like 0 or null refuse to run. - workflow update reports 'No workflows were eligible for update' when every target was skipped instead of claiming all are up to date. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): guard non-string catalog URL and keep enable/disable rollback intact - A truthy non-string catalog url (e.g. 123) reached urlparse and raised AttributeError, escaping the clean error path; validate it is a string. - enable/disable mutated the live registry entry before add(), so add's rollback snapshot captured the already-toggled object; pass a fresh mapping instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): tolerate non-dict registry entries in add and clarify test docstrings A corrupted-but-parseable registry entry (e.g. a string value) crashed WorkflowRegistry.add with AttributeError on existing.get. Guard the non-dict case while still restoring the original raw value on rollback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): atomic registry save and accurate mixed-target update summary - save() wrote the registry with open('w'), so a failed dump truncated the file and the next load reset every entry. Write to a sibling temp file and os.replace into place. - workflow update no longer claims all workflows are up to date when some targets were skipped; it reports checked-only status with a skipped count. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): exclusive temp file for registry save and cwd-independent disabled guard - save() now uses tempfile.mkstemp in the workflows dir (matching the engine's atomic writer), so a pre-created symlink at a predictable .tmp path cannot redirect the write and concurrent processes cannot collide. - The direct-path disabled guard derives the owning project from the resolved file path instead of the caller's cwd, so running an installed workflow's YAML from outside the project still refuses when disabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): symlink guards and shape validation in workflow registry, dev-dir file check - WorkflowRegistry now mirrors StepRegistry: _load refuses symlinked parents/registry file and normalizes a non-dict workflows field; save() rejects symlinked paths before writing. - workflow add --dev requires workflow.yml to be a regular file so a directory named workflow.yml gets the documented CLI error instead of an uncaught IsADirectoryError. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): validate download redirects before following them All three workflow download sites (add --from, catalog install, step install) passed no redirect_validator to open_url, so an HTTPS URL redirecting to cleartext HTTP issued the insecure request before the post-hoc geturl() check reported it. Shared validator now rejects non-HTTPS redirects (loopback HTTP allowed) pre-follow, matching the preset download path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(workflows): accept redirect_validator kwarg in step-add open_url fakes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): guard directory-shaped workflow.yml and unreadable registry - workflow add's plain local-path fallback (no --dev) checked wf_file.exists() before installing, so a directory literally named workflow.yml passed the guard and _validate_and_install_local() leaked an uncaught IsADirectoryError instead of the documented CLI error. Use is_file(), matching the --dev branch's existing guard. - WorkflowRegistry._load() treated any OSError while reading an existing registry the same as corrupted JSON, resetting to an empty in-memory registry. A later save() would then silently persist that empty state via os.replace, discarding every previously installed workflow entry. Track a _load_error flag on OSError-during-read and have save() refuse to write when it is set, so a transient I/O failure can no longer overwrite intact data on disk. - docs/reference/workflows.md: document `--from <url>` with its value placeholder, matching extensions.md and presets.md. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflow): rollback registry.remove() and guard OSError at CLI boundaries Critical: WorkflowRegistry.remove() deleted the in-memory entry then called save() with no rollback, unlike add(). Combined with workflow_remove deleting the workflow directory before calling registry.remove(), a save failure permanently destroyed the workflow's files, left the on-disk registry still claiming it installed, and surfaced a raw unhandled OSError with no CLI message. - WorkflowRegistry.remove() now rolls back the in-memory entry on a save() OSError, mirroring add()'s existing rollback pattern. - workflow_remove persists the registry removal (registry.remove(), wrapped in try/except OSError -> clean escaped message) before deleting any files, so a save failure never touches the workflow directory. Important sibling paths: workflow add (local/--dev/--from and catalog), enable, and disable all called registry.add() without catching its deliberate OSError, so a save failure surfaced either an orphaned install directory (fresh local/catalog installs) or a raw/unhandled exception with no clean CLI output. - _validate_and_install_local (backs local/--dev/--from) now removes the freshly created directory on a fresh install, or restores the prior workflow.yml bytes on a reinstall-over-existing-local install, before raising a clean escaped error. - _install_workflow_from_catalog wraps the final registry.add() using the function's own established convention (rmtree the just-downloaded workflow_dir, then a clean escaped error) -- workflow_update's existing backup/restore around this function is unaffected. - workflow_enable/workflow_disable catch registry.add()'s OSError and print a clean escaped message instead of leaking the exception. Added failing-first tests proving each behavior (registry-unit rollback test, CLI-level remove/add/enable/disable save-failure tests parametrized where they share one root cause), all confirmed red before the fix and green after. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflow): preserve prior catalog install on reinstall registry-save failure _install_workflow_from_catalog's final registry.add() failure handler unconditionally rmtree'd workflow_dir. That's safe for a brand-new install, but plain `workflow add <catalog-id>` also allows re-adding an already-installed workflow, downloading the new version over the existing directory first. If registry.add() then failed to save, the unconditional rmtree deleted the prior working install while the registry (after its own rollback) still reported it installed -- data loss with no way back. workflow_update already avoids this via an outer backup/restore around this function, but plain add has no such caller. Fix mirrors _validate_and_install_local's existed-before/backup-aware handling: capture whether workflow_dir existed and back up its workflow.yml bytes before any download write, then on a registry.add() OSError, restore those bytes for a reinstall or rmtree only a brand-new directory. Only one file (workflow.yml) is ever written by this path, so no further per-file bookkeeping is needed. Added a failing-first regression: install a catalog workflow, re-add it with a simulated registry save OSError, and assert a clean error, the original workflow.yml restored byte-for-byte, and the registry still reporting the original version installed. Confirmed red (prior file deleted) before the fix, green after. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflow): centralize catalog-install cleanup across all failure branches _install_workflow_from_catalog is new in this PR and has seven failure branches after the mkdir/download step, each independently rmtree'ing workflow_dir: redirect-to-non-HTTPS rejection, a generic download exception, invalid downloaded YAML, a validate_workflow failure, a workflow-id/catalog-key mismatch, a version mismatch, and (fixed in the prior commit) a registry.add() OSError. Only the last one had been special-cased to spare a prior working install on reinstall; the other six still unconditionally deleted the whole directory, so re-adding an already-installed catalog workflow and hitting any of those six earlier failures destroyed the working install even though nothing about it had actually changed. Replaced all seven ad hoc rmtree call sites with a single local _cleanup_failed_install() helper that closes over the existed_before / prior_workflow_bytes captured once at the top of the function: restore the prior workflow.yml for a reinstall, or rmtree only a directory that this attempt itself created. Every failure branch now calls this one helper, so the fix is structural rather than duplicated, and every existing error message/exit code is unchanged -- only the cleanup performed before each message is different. Added a parametrized regression test covering the four early-failure trigger points reachable from plain workflow add (redirect rejection, download exception, invalid YAML, ID mismatch): each installs a catalog workflow, re-adds it while forcing that specific failure, and asserts a clean error plus the original workflow.yml surviving byte-for-byte. Confirmed red against the unfixed code (all four raised FileNotFoundError reading the deleted file) before applying the helper, green after. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflow): restore registry entry verbatim on post-removal rmtree failure workflow_remove now persists registry.remove() before deleting any files (fixed previously), but if the registry write succeeds and the subsequent shutil.rmtree(workflow_dir) then fails, the registry was left claiming the workflow uninstalled while its directory remained on disk -- an orphaned install with no path back to a clean state. workflow_step_remove already handles this exact sequencing by capturing the registry entry before removal and restoring it directly into registry.data plus save() (bypassing add(), which would stamp a new updated_at) if the directory removal fails afterwards. Applied the same pattern to workflow_remove: capture registry_metadata via registry.get() before registry.remove(), and on an rmtree OSError, write it straight back into registry.data["workflows"][workflow_id] and save(), matching workflow_step_remove's restore-failure handling (a yellow warning, not a hard failure, since the primary error is already about to be reported). Existing error message and exit behavior for the rmtree failure are unchanged. Added a failing-first regression: install a workflow, monkeypatch shutil.rmtree to raise OSError, and assert a clean existing error message, the directory remaining (rmtree never actually deleted anything), and the registry entry restored byte-for-byte identical (including installed_at/updated_at) -- proving the fix bypasses add() and doesn't re-stamp timestamps. Confirmed red (registry entry stayed None) before the fix, green after. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix 4 current Copilot review findings on workflow run/registry/install 1. workflow run ownership check followed symlinks via Path.resolve() before mapping a direct YAML path back to its installed workflow ID. A symlinked .specify/workflows/<id>/workflow.yml resolved outside the tree, missed the ownership match entirely, and let the disabled-workflow guard be silently skipped while engine.load_workflow still followed the symlink. Now maps ownership from a lexically-normalized path (os.path. normpath, no symlink following) and explicitly refuses to run if the installed <id> directory or workflow.yml leaf is itself a symlink. Direct external workflow paths that don't match .specify/workflows/... are unaffected. 2. WorkflowRegistry._load() caught a read OSError and silently fell back to an empty in-memory registry, only blocking a later save(). Callers that only query is_installed()/get()/list() before writing a file (e.g. commands/init.py's bundled speckit install, which overwrites workflow.yml once is_installed() reports false) could act on that false-empty state and destroy real data before ever reaching save(). _load() now raises OSError immediately so an unreadable registry fails closed at construction, before any query or side effect is possible. Added _open_workflow_registry() to give every CLI command a consistent clean-error boundary around registry construction. 3. _validate_and_install_local's mkdir/copy2 ran before the try/except that protected registry.add(); a copy2 failure (e.g. a truncating partial write on a reinstall) was not caught at all, so the existing backup-restore cleanup never ran and the prior working workflow.yml was corrupted with a raw traceback surfaced to the user. mkdir/copy2 now run inside the same rollback-protected section as registry.add(), sharing one _cleanup_failed_install() helper. 4. workflow update's skip message claimed any non-catalog source was installed "from a local path or URL", which is wrong for the bundled speckit workflow (source: "bundled"). Message is now source-neutral. Verified all 4 threads are current (not outdated) via GraphQL review thread query on PR #3419, HEAD 812050a. Tests: strict TDD per fix (red test proving each bug, minimal production change, green). tests/test_workflows.py: 474 passed. Full suite: 3976 passed, 110 skipped. ruff check: all checks passed on touched files and full src tree. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix disabled-workflow bypass via symlinked .specify project root workflow run's ownership check derived registry_root/registered_id from the lexical path, then checked the id directory and workflow.yml leaf for symlinks -- but never checked .specify or .specify/workflows themselves for that derived root. _reject_unsafe_workflow_storage only guards the cwd's project_root, which can differ from the path-derived registry_root (a direct path into an unrelated project, or that project's own .specify being a symlink to an attacker-controlled tree). WorkflowRegistry's own symlinked-parent handling silently substitutes an empty registry instead of raising, so a query against it (is_installed/get returning "not found") is not a safety signal a caller can rely on: with a symlinked .specify, the disabled check saw no registry entry and let a disabled workflow run anyway. Fix: reject an unsafe .specify/.specify-workflows for the actual derived registry_root before ever consulting the registry, reusing the existing _reject_unsafe_dir helper already used by _reject_unsafe_workflow_storage. Red-first end-to-end repro: victim project's .specify symlinked to an attacker-controlled tree containing a disabled workflow entry, run invoked with a direct path from an unrelated cwd -- confirmed the disabled workflow executed (exit 0) before the fix, now refused cleanly. Tests: tests/test_workflows.py 475 passed. Full suite: 3977 passed, 110 skipped. ruff check: all checks passed. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix raw exception leak in bundle remove primitive boundary remove_bundle() had no exception handling around its component removal loop, unlike install_bundle() which converts any raw exception into a clean BundlerError. Since WorkflowRegistry now fails closed (raises OSError) on an unreadable registry file, and _WorkflowKindManager.__init__ constructs WorkflowRegistry with no try/except, an unreadable workflow registry surfaced as a raw OSError through remove_bundle(). The bundle_remove CLI command only catches BundlerError, so the raw OSError propagated uncaught, producing exit_code=1 with empty output instead of a clean, actionable message. Wrap remove_bundle()'s component loop in the same try/except BundlerError: raise / except Exception: raise BundlerError(...) from exc pattern already used by install_bundle(), converting any raw exception at this shared boundary. save_records() remains outside the try block, so a failure still leaves the bundle's record untouched (no removal side effects recorded). Tests: - tests/integration/test_bundler_install_flow.py::test_remove_converts_raw_installer_exception_to_bundler_error (function-level regression: a raw OSError from installer.is_installed must become a clean BundlerError, and the bundle record must survive) - tests/contract/test_bundle_cli.py::test_remove_reports_clean_error_when_primitive_raises_raw_exception (CLI-level regression: `specify bundle remove` must print a clean actionable message and exit non-zero instead of raw/empty output) Both tests were confirmed red beforehand: the raw OSError propagated uncaught out of remove_bundle(), and the CLI-level CliRunner result showed exit_code=1 with empty output. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix 8 current Copilot review findings on registry fail-closed, rollback orphans, backup-read boundaries, and Rich escaping 1. WorkflowRegistry._load(): a symlinked .specify/.specify/workflows parent (or a symlinked registry file) silently returned an empty registry instead of raising, unlike an unreadable-file read failure. A read-only caller (notably the bundler's remove path) querying is_installed() before ever writing could conclude an installed workflow is absent, skip removing it, then delete the bundle record -- leaving the workflow untracked but still on disk. Now raises OSError immediately, matching the existing unreadable-file fail-closed behavior. 2/8. _validate_and_install_local and _install_workflow_from_catalog: when the destination directory already existed but had no prior workflow.yml (e.g. a leftover empty dir), existed_before was True but there were no backup bytes to restore, so the rollback closure did nothing on a later failure -- leaving the newly copied/ downloaded file behind. Both now unlink the newly created file in this case, restoring the pre-existing directory to its prior (empty) state. 3/4. Both install paths read the prior workflow.yml bytes (to seed the reinstall rollback) *before* any try/except boundary: a read failure on the existing file (e.g. a transient permission/FS issue) leaked a raw, unescaped OSError instead of the same clean CLI error used by every other failure branch in these functions. Both reads are now guarded by their own try/except OSError, with no writes attempted before the read succeeds (so there is nothing to roll back on this specific failure). 5. remove_bundle's exception-conversion message unconditionally claimed "No changes were recorded," even though a failure can occur after earlier components in the same bundle have already been removed from disk (save_records never runs on this path, so the record is left claiming the bundle fully installed). The message now reports how many components were already removed when that happened, instead of asserting no changes occurred. 6/7. workflow_remove's new post-registry-removal directory-failure error and its restore-failure warning interpolated workflow_dir and the exception values into Rich markup unescaped. A project path or OS error message containing Rich-markup-like brackets could be parsed as markup and hide/corrupt the displayed text. Both now use the existing _escape_markup helper, consistent with every other error path in this file. Tests (tests/test_workflows.py unless noted): - TestWorkflowRegistry::test_load_symlinked_workflows_dir_fails_closed_not_silently_empty (1) - TestWorkflowCliAlignment::test_add_dev_fresh_install_into_preexisting_empty_dir_cleans_new_file (2) - TestWorkflowCliAlignment::test_add_catalog_fresh_install_into_preexisting_empty_dir_cleans_new_file (8) - TestWorkflowCliAlignment::test_add_dev_reinstall_backup_read_failure_gives_clean_error (3) - TestWorkflowCliAlignment::test_add_catalog_reinstall_backup_read_failure_gives_clean_error (4) - tests/integration/test_bundler_install_flow.py::test_remove_partial_failure_message_reflects_partial_state (5) - TestWorkflowRemoveGuard::test_remove_directory_and_restore_failure_escapes_rich_markup (6/7) All seven were confirmed red beforehand, matching each thread's described failure mode exactly (silent empty registry instead of a raise; orphaned new file left behind; raw unescaped OSError leaking; a misleading "no changes were recorded" claim; Rich markup consuming bracketed path/exception text). Also updated test_registry_save_refuses_symlinked_parent, a pre-existing test that asserted the symlinked-parent raise at add()/save() time -- it now raises at construction instead, per fix #1, so the test was adjusted to match without weakening its guarantee (still asserts no writes occur under the symlinked target). Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix 3 current Copilot review findings: bookkeeping-aware BundlerError removal, bounded workflow downloads 1. bundle remove: BundlerError raised by the primitive installer itself (e.g. from a kind manager) bypassed the partial-removal bookkeeping message added previously via a bare `except BundlerError: raise`. Now routes through the same detail-construction logic as generic exceptions, so a mid-loop BundlerError after an earlier successful removal still reports that the project may be partially uninstalled, while a zero-removal BundlerError still reports "No components were removed." Both preserve the original exception message and chain `from exc`. 2/3. workflow add --from and catalog install/update downloads used unbounded `response.read()`, buffering the entire server-controlled body into memory before any size check, and trusted Content-Length alone where checked at all. Added a single shared `_read_response_within_limit()` helper reused by both call sites: it fails fast on an oversized declared Content-Length, and separately enforces the same cap while streaming in 64KiB chunks so a chunked or Content-Length-less response cannot bypass the limit by lying about or omitting its size. Chose 5 MiB as the cap: workflow YAML definitions are small step/metadata text, not binaries, so this is generous headroom against a malicious/misbehaving server without affecting any legitimate workflow definition. Both call sites already route any raised exception through their existing clean-error and rollback (`_cleanup_failed_install`) paths, so no additional error-handling plumbing was needed. Tests: extended the shared `_FakeResponse` test helper (and 5 duplicate per-test FakeResponse classes) to support `.read(amt)` chunked reads with an internal cursor (backward compatible with existing bare `.read()` callers) plus header simulation. Added red-first tests for: BundlerError after partial removal reporting partial state, BundlerError with zero removals reporting no changes, --from oversized-Content-Length rejection, --from oversized-streamed-body-without-Content-Length rejection, and the same two cases for the catalog install path (asserting no orphan directory/registry mutation on rejection). tests/integration/test_bundler_install_flow.py: 17 passed tests/test_workflows.py: 485 passed tests -q: 3992 passed, 110 skipped ruff check: clean on all touched files Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix temp-file leak in workflow add --from and strengthen size-limit test assertions workflow_add's --from download path opened a NamedTemporaryFile(delete=False) -- which creates the file on disk immediately -- then wrote the size-limited response body before assigning `tmp_path`. If `_read_response_within_limit` raised (oversized declared Content-Length, or an over-cap streamed body with no/understated Content-Length), the exception propagated out of the `with` block before `tmp_path` was ever set, so the outer except handler had no path to clean up: a 0-byte `.yml` temp file was left behind permanently on every rejected/failed --from download. Fixed by assigning `tmp_path` immediately after the file is opened (before the size-limited read/write), and unlinking it in the except branch when set. Normal post-download cleanup in the existing `finally: tmp_path.unlink(missing_ok=True)` is unchanged. Verified (not assumed) the catalog install path has no equivalent leak: it writes the response bytes directly to `workflow_file` inside `workflow_dir` (no separate temp file), and any read/size-limit failure is already caught by the existing `except Exception: _cleanup_failed_install()` handler, which correctly restores a reinstalled file or removes a freshly-created directory. While investigating, found the previous round's 4 size-limit tests were false positives: `_read_response_within_limit`'s `max_bytes` parameter had its default bound to `_MAX_WORKFLOW_YAML_BYTES` at function-definition time, so monkeypatching the module attribute in tests had no effect on the function's actual behavior -- the tests were passing because the oversized mock bodies failed downstream YAML/id validation instead of the size check. Fixed by resolving `max_bytes` from the module attribute at call time (default `None`, resolved inside the function body) so tests can actually override the effective limit, and strengthened all 4 tests' assertions to match the specific size-limit error text (whitespace-collapsed to tolerate Rich's line-wrapping), so they now prove the real code path fires. Tests: added 2 red-first regression tests (oversized-streamed-body and oversized-Content-Length --from downloads leave no leftover temp file, verified against a scratch tempfile.tempdir), confirmed red (real 0-byte file found) before the fix and green after. Strengthened the pre-existing 4 --from/catalog size-limit tests to assert on the actual error message instead of generic exit-code/non-empty-output checks. tests/test_workflows.py: 487 passed tests -k bundler: 186 passed tests -q: 3994 passed, 110 skipped ruff check: clean on all touched files Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden workflow install/remove transactions with atomic staging Addresses 5 Copilot review findings on HEAD b8269c8, all centered on transaction integrity around workflow install/remove/registry writes, following the atomic_write_json pattern already used in _utils.py: 1. WorkflowRegistry.save() now preserves the existing registry file's mode (e.g. 0640/0644) across a save instead of silently downgrading it to mkstemp's 0600 default; a brand-new registry still gets the secure 0600 default. 2. workflow_remove now stages the install directory out of the way via an atomic rename *before* the registry write, rather than deleting it directly with shutil.rmtree after the registry already claims it removed. This closes a real data-integrity gap: a partially-failed rmtree could no longer leave a damaged directory re-marked "installed" by the old manual restore-after-rmtree-failure code (now deleted -- it's structurally impossible to need it). A registry-write failure renames the staged directory back (guarded, with an explicit warning if the restore-back rename itself fails); a registry-write success is durable, so a later failure to delete the staged directory is now a warning (exit 0), not a contradictory "Error: Failed to remove" (exit 1) that used to claim failure while the registry already recorded success. 3. Local (--dev/--from/plain path) and catalog install/reinstall now write new content to a same-directory staging file and commit it onto the destination workflow.yml via a single atomic swap, instead of writing/downloading directly into the destination file. A prior file (reinstall) is renamed aside rather than overwritten in place, so it can be restored via rename -- never a content rewrite -- if registry.add() subsequently fails; a rollback failure is now explicitly reported as a warning instead of escaping unguarded and masking the original clean error. This also removes the need to read the prior file's bytes into memory before installing (that read-before-write step and its failure mode are now unreachable), and both local and catalog installs share the same four small helpers (_stage_workflow_file / _commit_workflow_file / _discard_staged_workflow_file / _rollback_committed_workflow_file, plus guarded wrappers) rather than duplicating the logic. 4. Updated a stale comment (workflow_run's ownership-guard rationale) that still described WorkflowRegistry._load() as silently substituting an empty registry; it now fails closed by raising OSError, which the comment now states plainly. Tests: rewrote the two workflow_remove tests whose assertions encoded the old (incoherent) rmtree-then-restore contract to instead prove the new stage-then-commit contract (post-registry-success cleanup failure is a warning+exit 0; pre-registry-success stage-restore failure is guarded and escapes markup correctly). Rewrote the local/catalog "backup read failure" tests, which tested a step the new design no longer performs, into "restore-rename failure" tests proving the new guarded rollback boundary. Added registry file-mode preservation tests. All other existing install/remove/reinstall tests (save-failure cleanup, pre-existing-empty-dir handling, early-failure-during- reinstall parametrized cases, Rich markup escaping) continue to pass unmodified against the new implementation. Verified via GraphQL that all 5 threads are current (not outdated/ resolved) before fixing. Full suite: 3996 passed, 110 skipped. Ruff clean on all touched files. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Discard reinstall backup file after registry.add() succeeds _commit_workflow_file() renames a prior workflow.yml aside to workflow.yml.bak so it can be restored if registry.add() subsequently fails. Neither the local install/reinstall path nor the catalog install/reinstall path ever cleaned up that backup after a successful registry.add() -- every successful reinstall permanently left a workflow.yml.bak sibling, which later reinstalls would silently overwrite/re-orphan. Add a shared _discard_committed_backup_file() helper, called from both success paths right after registry.add() durably succeeds (and before the final "installed" message, preserving output ordering). A fresh install (backup_file is None) is a no-op. A cleanup failure is reported as a warning (exit 0), not a failure, since the install itself already succeeded -- consistent with workflow_remove's post-commit cleanup warning semantics. Add red-first regression tests proving: (1) successful local reinstall leaves no workflow.yml.bak sibling, (2) successful catalog reinstall leaves no workflow.yml.bak sibling, (3) a cleanup failure on the backup file after a successful reinstall reports a warning and still exits 0 with the registry correctly reflecting the new install. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clean up freshly-created dest_dir when staging mkstemp fails _stage_workflow_file() does dest_dir.mkdir(parents=True, exist_ok=True) then tempfile.mkstemp(dir=dest_dir, ...). For a fresh install (no prior directory), if mkdir succeeds but mkstemp then raises (disk full/EMFILE/quota), the exception previously propagated straight past both the local-install and catalog-install call sites without any cleanup, leaving the newly-created empty workflow directory orphaned on disk with no error indicating why. Fix at the shared _stage_workflow_file() boundary instead of duplicating cleanup at each call site: track whether this call created dest_dir: on a mkstemp failure, remove that directory via a guarded rmdir (never a broad rmtree, so any concurrently written content would be left untouched) before re-raising the original OSError unchanged. A pre-existing (reinstall) dest_dir is never touched by this cleanup, and a cleanup failure is reported as its own warning without masking the original error. Add red-first regression tests proving: a fresh local install (--dev, plain local path, --from) and a fresh catalog install both clean up the orphaned directory on a simulated mkstemp failure, and a reinstall over a pre-existing directory is left untouched by the same failure. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix installed-workflow ownership/disabled bypass and resume enforcement Address 3 current Copilot review findings on the disabled-workflow guard in `workflow run`/`workflow resume`: - The lexical `.specify/workflows/<id>` ownership scan stopped at the first match scanning from the start of the path. A nested project living beneath an outer installed workflow's own directory tree (reusing the same segment names) was attributed to the wrong (outer) workflow and ID, gating the run on an unrelated workflow's disabled state. `_scan_for_workflow_owner` now scans from the end so the nearest (innermost) owner always wins. - A path with no `.specify/workflows` segments of its own (e.g. `/tmp/alias.yml`) that is itself a symlink resolving *into* installed storage bypassed the disabled check entirely, since only the raw lexical path was inspected. `_resolve_installed_workflow_ownership` now additionally resolves the real path when the lexical scan finds no owner and re-runs the same scan against it, so an outward-pointing alias into a disabled workflow is caught too. Genuinely standalone external files (no symlink anywhere on the path) are unaffected. - `workflow resume` bypassed the disabled check altogether: engine.resume() replays a persisted run directly from disk with no registry awareness. RunState now optionally persists `installed_workflow_id` and `installed_registry_root` at run start (set by workflow_run when the source resolved to an installed ID); `workflow_resume` pre-loads the run state and re-checks the registry's *current* disabled state before calling engine.resume(), mirroring workflow_run's own guard. Both new fields default to None via RunState.load()'s `.get()`, so runs from a direct/non-installed source, and any run persisted before this schema addition, resume exactly as before. The ownership-mapping logic (previously inlined in workflow_run) is extracted into `_resolve_installed_workflow_ownership` / `_scan_for_workflow_owner` so both the lexical and resolved-path cases share the same scan and the existing inward-symlink-component refusal. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Guard --from temp cleanup; drop redundant update rollback; mark POSIX-only tests Two more current Copilot review findings, both in workflow_add/update: - `workflow add --from`'s `finally: tmp_path.unlink(missing_ok=True)` ran unguarded after `_validate_and_install_local` had already committed the file and registry entry (success) or already raised its own clean `typer.Exit` (failure). An OSError from that cleanup unlink would surface as an unhandled failure even though the install itself succeeded. It is now wrapped in try/except OSError, printing a neutral warning that doesn't claim success or failure (the finally runs on both outcomes) instead of propagating. - `workflow_update`'s per-item loop performed its own outer backup (`wf_file.read_bytes()`) and restore (`wf_file.write_bytes(backup)`) around `_install_workflow_from_catalog`, which is itself fully transactional (staged download, atomic rename-based commit, its own rollback on registry failure) and never leaves a raw OSError or a partially-written workflow.yml. The outer restore was therefore dead weight for its stated purpose, and — being an unguarded byte-level write — was itself an unnecessary place a second failure could truncate an already-safely-preserved file. Removed; the loop now only records success/failure. Also marks 3 registry-save file-mode tests (`test_registry_save_preserves_existing_file_mode`, `test_registry_save_on_new_registry_uses_secure_default_mode`, `test_registry_save_failure_preserves_file_on_disk`) as POSIX-only via the repo's existing `skipif(sys.platform == "win32", ...)` pattern, since they assert exact POSIX permission bits that don't hold on Windows. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Report possible partial changes on zero-removed bundle removal failure The final Copilot review finding: `remove_bundle`'s zero-removed-components error message claimed "No components were removed." even when the failing installer component may have deleted files before raising -- prior review rounds already established that DefaultPrimitiveInstaller's removal paths are not atomic and can leave partial filesystem changes despite raising before `result.uninstalled` is populated. The zero-count message is now a conservative caution ("...but the failing component may have made partial changes before raising, so the project may be partially uninstalled.") instead of an unconditional claim of no side effects. The >0-removed path (which already reports the confirmed partial list) is unchanged. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix workflow resume disabled-check bypass after project move/rename RunState.installed_registry_root previously persisted the creation-time absolute project path unconditionally whenever a run belonged to an installed workflow. After the whole project directory was renamed or moved, workflow_resume would open a WorkflowRegistry at that now nonexistent path, get back an empty/default registry, and silently skip the disabled-workflow check -- a paused run for a disabled workflow could be resumed successfully from the new location. Fix persists installed_registry_root only when the owning root genuinely differs from the current project_root (true cross-project direct-file- source invocations). The common same-project case now persists None and is re-derived from the live project_root at resume time via a new _resolve_run_owner_root() helper, which also falls back to project_root if a stored root no longer exists on disk -- covering both the common case transparently surviving project moves and the cross-project case degrading safely if its owner project vanishes, rather than silently skipping the disabled check. Backward compatible: state files missing the new fields, and states with a still-existing distinct cross-project root, behave unchanged. Added regression tests: - resume blocked after project moved then disabled at new location - resume still works after project moved while workflow stays enabled - cross-project registry root is still correctly honored when it exists - resume falls back to current project's registry when a stored cross-project root no longer exists Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix silenced cleanup failures and malformed run-state type validation Four fixes from Copilot review on HEAD 4f24735: 1. _discard_staged_workflow_file's fresh-install directory removal used shutil.rmtree(dest_dir, ignore_errors=True), so a genuine cleanup failure there could never reach _safe_discard_staged_workflow_file's warning -- an orphaned directory was left behind with zero report. Now removes only if dest_dir still exists and lets a real OSError propagate to the existing safe wrapper, which warns while the already-printed original install error remains primary. 2. _rollback_committed_workflow_file's fresh-install directory removal (post registry.add() failure) had the same ignore_errors=True gap; fixed identically so _safe_rollback_committed_workflow_file's warning can actually fire. 3. In the --from download-failure branch, tmp_path.unlink(missing_ok= True) was unguarded: if it raised (e.g. read-only tempdir), it replaced the original "Failed to download workflow" error with a raw unhandled OSError instead of a clean typer.Exit. Now guarded exactly like the later post-install finally cleanup: a cleanup failure prints a warning and the original download error is still reported cleanly. 4. RunState.load() trusted installed_workflow_id/installed_registry_root straight out of state.json with no type validation. A malformed value (int/list/dict/bool instead of str-or-null) would crash deep inside _resolve_run_owner_root or the registry lookup (TypeError building a Path, unhashable dict/list as a mapping key) instead of failing cleanly. Both fields are now validated as str | None during load, raising a clear ValueError that workflow_resume's existing ValueError boundary already converts into a clean CLI error with no traceback. Valid values (including the empty-string fallback already handled by _resolve_run_owner_root) continue to load unchanged. Added red-first regression tests for each: staged-discard cleanup warning, rollback cleanup warning, download-failure cleanup-vs-original- error precedence, and parameterized malformed/valid run-state field coverage. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add ValueError boundary to workflow status single-run lookup QUALITY re-review flagged that fa410d3's new RunState.load() type validation (malformed installed_workflow_id/installed_registry_root raising ValueError) leaked as a raw unhandled traceback through `workflow status <run_id>`, which only caught FileNotFoundError. `workflow resume` already had the matching ValueError boundary. Adds an `except ValueError as exc: console.print(f"[red]Error:[/red] {exc}"); raise typer.Exit(1)` clause mirroring resume's exact pattern (unescaped interpolation, consistent with the existing convention at every other ValueError boundary in this file). FileNotFoundError behavior and the no-run-id list-all-runs path are unchanged. Added parametrized regression covering malformed installed_workflow_id/ installed_registry_root (int/list) via `workflow status`, plus regressions locking in the unaffected FileNotFoundError and no-run-id list-path behaviors. Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: bound workflow step downloads Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: preserve workflow reinstall state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: fail closed on workflow registry state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: make workflow installs transactional Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: close workflow transaction races Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: clean up failed workflow transactions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: clean up workflow removal state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: guard workflow update transactions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: harden workflow lifecycle edge cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: verify installed workflow ownership Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: isolate workflow rollback cleanup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: preserve unique workflow backups Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: bind workflow staging to file descriptors Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: fail closed on corrupt workflow registry Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: bind workflow ownership and source identity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): harden redirects and Windows tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): restore staged removals on serialization errors Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): preserve state across interrupted writes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): harden resume ownership checks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): validate persisted run state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(workflows): validate origin and release metadata Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add John Lam as contributor and release badge

084d111

- Add John Lam (@jflam) to maintainers section - Include GitHub Actions release workflow badge in header - Update README formatting for better visibility

Copilot AI review requested due to automatic review settings August 25, 2025 18:26

Copilot AI reviewed Aug 25, 2025

View reviewed changes

localden merged commit dc6556d into main Aug 25, 2025
4 checks passed

localden deleted the update-readme-contributors branch August 25, 2025 21:01

tiwillia added a commit to tiwillia/spec-kit that referenced this pull request Sep 9, 2025

Merge pull request github#1 from tiwillia/agents_context

e50eb5a

Add AGENTS.md contextual file

efe-mumoglu mentioned this pull request Sep 10, 2025

Bug noticed when running scripts/update-agent-context.sh #47

Closed

mfeeney mentioned this pull request Sep 16, 2025

update-agent-context.sh script fails with sed expression errors and incorrect template path #293

Closed

localden pushed a commit that referenced this pull request Sep 24, 2025

Merge pull request #1 from danyow/patch-1

04cb5f0

Update template path for spec file creation

fergoid mentioned this pull request Sep 26, 2025

what if one misses a step #541

Closed

jellydn pushed a commit to jellydn/spec-kit that referenced this pull request Sep 30, 2025

Merge pull request github#1 from github/update-readme-contributors

0cb1a41

Add John Lam as contributor and release badge

jellydn pushed a commit to jellydn/spec-kit that referenced this pull request Sep 30, 2025

Merge pull request github#1 from danyow/patch-1

a27018b

Update template path for spec file creation

acomito mentioned this pull request Oct 25, 2025

Add ability to spawn worktree when creating new branch #61

Closed

elTorres pushed a commit to elTorres/spec-kit that referenced this pull request Nov 5, 2025

Merge pull request github#1 from mindblowngaming/imgbot

15eaff5

[ImgBot] Optimize images

jonshaffer mentioned this pull request Nov 21, 2025

Feedback from production migration: branch validation bug and documentation inconsistencies #1232

Closed

elTorres pushed a commit to elTorres/spec-kit that referenced this pull request Nov 26, 2025

Merge pull request github#1 from danieldekay/copilot/fix-branch-numbe…

c78c3d7

…ring-issue Fix branch numbering to increment globally across all features

FidelusAleksander mentioned this pull request Nov 30, 2025

copilot-instructions.md created in the wrong folder #1254

Closed

sarmadalikhanofficial pushed a commit to syrextech/spec-kit-plus that referenced this pull request Dec 3, 2025

Merge Qwen Code CLI: Hands Advanced Modules (MCP, Subagents, Evals, C…

b809b87

…heckpointing, Vision) pull request github#1 from panaversity/ai_assistant_mastery

This was referenced Mar 18, 2026

chore: update DocGuard extension to v0.9.10 #1888

Closed

chore: update DocGuard extension to v0.9.10 #1890

Merged

mnriem mentioned this pull request Mar 20, 2026

From Agents to Integrations #1924

Closed

6 tasks

Copilot AI mentioned this pull request Mar 20, 2026

Stage 1: Integration foundation — base classes, manifest system, and registry #1925

Merged

mnriem mentioned this pull request Mar 23, 2026

Extract Git/Branching Operations into extensions/git Extension (Opt-Out, Auto-Enabled) #1940

Closed

23 tasks

Copilot AI mentioned this pull request Mar 23, 2026

feat: Git extension stage 1 — bundled extensions/git with hooks on all core commands #1941

Merged

Quratulain-bilal mentioned this pull request Apr 8, 2026

Add Spec Refine community extension to catalog and README #2118

Merged

7 tasks

mnriem mentioned this pull request Apr 15, 2026

feat: implement preset wrap strategy #2189

Merged

5 tasks

janwillemkeizer mentioned this pull request May 23, 2026

Shell scripts: paths-only validates branch and setup-plan can overwrite plan.md #2653

Closed

raccioly mentioned this pull request Jun 11, 2026

[Extension]: Update DocGuard — CDD Enforcement (v0.26.0) #2928

Closed

11 tasks

raccioly mentioned this pull request Jun 22, 2026

[Extension]: Update DocGuard — CDD Enforcement (v0.27.0) #3093

Closed

11 tasks

mnriem mentioned this pull request Jun 22, 2026

feat: make agent-context extension a full opt-in #3097

Merged

fuleinist mentioned this pull request Jun 23, 2026

feat(workflows): add --dry-run flag to specify workflow run #2704

Closed

mnriem mentioned this pull request Jul 13, 2026

fix: rewrite extension skill command references #3474

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add John Lam as contributor and release badge#1

Add John Lam as contributor and release badge#1
localden merged 1 commit into
mainfrom
update-readme-contributors

localden commented Aug 25, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

localden commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

localden commented Aug 25, 2025 •

edited

Loading