diff --git a/.changeset/159-research-phase-auto-use-existing.md b/.changeset/159-research-phase-auto-use-existing.md deleted file mode 100644 index 0d0d6dbee..000000000 --- a/.changeset/159-research-phase-auto-use-existing.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Changed -pr: 718 ---- -`/gsd:plan-phase --research-phase ` now auto-uses an existing `RESEARCH.md` instead of prompting update/view/skip. When research already exists and neither `--research` nor `--view` is passed, it emits a one-line notice and exits cleanly, matching the promptless behavior of standard `/gsd:plan-phase `. Pass `--research` to force-refresh or `--view` to print the existing research. (#159) diff --git a/.changeset/56-retire-legacy-runtime-directory-helpers.md b/.changeset/56-retire-legacy-runtime-directory-helpers.md deleted file mode 100644 index 4737ef30f..000000000 --- a/.changeset/56-retire-legacy-runtime-directory-helpers.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -type: Changed -pr: 802 ---- -Retire the installer's one-off runtime directory helpers (`getGlobalDir`/`getOpencodeGlobalDir`/`getKiloGlobalDir`) and consolidate per-runtime global config-dir resolution onto the single canonical projection `runtime-homes:getGlobalConfigDir`, extended with the `--config-dir` override and the opencode/kilo `*_CONFIG` file-path precedence. Behavior-preserving across all 15 install runtimes. (#56) - - diff --git a/.changeset/60-runtime-config-adapter-registry.md b/.changeset/60-runtime-config-adapter-registry.md deleted file mode 100644 index 802af38a6..000000000 --- a/.changeset/60-runtime-config-adapter-registry.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -type: Changed -pr: 795 ---- -Make per-runtime config-mutation dispatch in the installer explicit: a new runtime config adapter registry maps each supported runtime to a typed config intent (install surface, shared-settings gate, finish-phase permission writer), and `install()`/`finishInstall()` dispatch by resolved intent instead of inline `runtime === '...'` branching. Behavior-preserving; unknown runtimes now fail loudly. (#60) - - diff --git a/.changeset/651-verification-status-consolidation.md b/.changeset/651-verification-status-consolidation.md deleted file mode 100644 index 166f1b729..000000000 --- a/.changeset/651-verification-status-consolidation.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Changed -pr: 755 ---- -**Verification status routing is now owned by a single queryable seam** — `ship.md` and `execute-phase.md` both consume `gsd_run query verification.status` instead of re-deriving the `passed`/`gaps_found`/`human_needed` routing independently; the query returns `next_action` and `next_command` so per-status prose no longer needs to be kept in sync across files. This also fixes the broad-grep status misread in `execute-phase.md` where a body `status:` line (in a code block or copied artifact) could concatenate with the frontmatter value and misroute a valid passed phase; a parity test fails if a new verifier status value lacks a route. (#651) diff --git a/.changeset/656-research-module-seam.md b/.changeset/656-research-module-seam.md deleted file mode 100644 index b086cea04..000000000 --- a/.changeset/656-research-module-seam.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 664 ---- -**Research is now cached, curated-first, and code-governed** — a content-addressed Research Store (per-source TTL), a single provider waterfall with confidence tiers, and registry-API package legitimacy replace the per-agent prose waterfall and the slopcheck bolt-on. (#664) Confidence is now verification-evidence-driven: provider identity alone no longer yields HIGH; HIGH requires ground-truth corroboration (e.g. `legitimacyVerdict: 'OK'`), authority alone caps at MEDIUM, and SLOP caps at LOW. diff --git a/.changeset/669-gsd-review-cursor-agent.md b/.changeset/669-gsd-review-cursor-agent.md deleted file mode 100644 index 3a3ac40ba..000000000 --- a/.changeset/669-gsd-review-cursor-agent.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 686 ---- -`/gsd-review --cursor` now actually invokes the Cursor agent. Detection probes the `cursor-agent` headless binary instead of the `cursor` IDE launcher, the invocation calls the single `cursor-agent` binary in print mode (not the two-token `cursor agent`, which the IDE treats as a file path), and the review prompt is passed as a file-path argument rather than piped to stdin (which `cursor-agent -p` ignores). On failure the captured stderr is surfaced instead of a silent empty result. diff --git a/.changeset/685-windowshide-spawn.md b/.changeset/685-windowshide-spawn.md deleted file mode 100644 index 997c18019..000000000 --- a/.changeset/685-windowshide-spawn.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 688 ---- -**No more "gsd-core" console-window flash on Windows.** Every gsd-core child process now passes `windowsHide: true`: the context monitor's `record-session` spawn, the `execGit` / `execNpm` / `execTool` helpers in `shell-command-projection`, the `gsd-worktree-path-guard` and `gsd-workflow-guard` hook git probes, `check-command-router`'s `git log` call, and the `roadmap-upgrade` git status/rev-parse/reset/clean calls — matching the existing `gsd-check-update` spawn. `execNpm` (which uses `shell: true` → `cmd.exe` and runs on every SessionStart, i.e. every `/clear`) and the worktree-path guard (which runs on every Edit/Write in a worktree) were the most visible offenders. No behavior change on macOS/Linux, where the flag is ignored. diff --git a/.changeset/687-agy-print-timeout.md b/.changeset/687-agy-print-timeout.md deleted file mode 100644 index ea9f4eb48..000000000 --- a/.changeset/687-agy-print-timeout.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 689 ---- -**`/gsd-review --agy` no longer hangs the whole review on large prompts.** On a big, file-path-rich prompt Antigravity's `agy -p` agentic Cascade can loop on its `code_search`/grep steps and never converge. The invocation now passes agy's own `--print-timeout` flag (its native print-mode cap) so a stalled run self-terminates through the tool's own mechanism; on a non-zero exit any partial output is discarded so the existing transcript fallback / "review failed" stub take over. diff --git a/.changeset/703-plan-phase-granularity-flag.md b/.changeset/703-plan-phase-granularity-flag.md deleted file mode 100644 index 74fe53f0d..000000000 --- a/.changeset/703-plan-phase-granularity-flag.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 750 ---- -`/gsd:plan-phase` now accepts a `--granularity ` flag to override the configured planning granularity for a single invocation. The flag takes precedence over `granularities.planning`, top-level `granularity`, and `planning.granularity` config. Invalid values are rejected. (#703) diff --git a/.changeset/730-roadmap-milestone-phase-details-scope.md b/.changeset/730-roadmap-milestone-phase-details-scope.md deleted file mode 100644 index ea32b54a1..000000000 --- a/.changeset/730-roadmap-milestone-phase-details-scope.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 748 ---- -The roadmap parser now resolves fresh phases of the current milestone in multi-milestone roadmaps. `extractCurrentMilestone()` scoped the current-milestone window to its `## Phases` checklist subsection and stopped at the milestone's own `## Milestone … (Phase Details)` heading, so the `### Phase N:` detail headers fell out of scope. Any command backed by the parser — `init.phase-op` (and therefore `/gsd:discuss-phase` and `/gsd:plan-phase`), `state`, `roadmap list`, and `validate health` (W006) — could not resolve phases of any milestone after the first until a `.planning/phases/` directory already existed, blocking discuss/plan. The parser now also includes the current milestone's `(Phase Details)` section in scope, anchored to the selected milestone's version token so sibling sub-milestones do not cross-pollinate. (#730) diff --git a/.changeset/751-config-prototype-pollution-codeql-alert-26.md b/.changeset/751-config-prototype-pollution-codeql-alert-26.md deleted file mode 100644 index 2e638c05e..000000000 --- a/.changeset/751-config-prototype-pollution-codeql-alert-26.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Security -pr: 752 ---- -**`gsd-tools config-set` prototype-pollution guard hardened and regression-tested.** The guard that blocks `__proto__`, `prototype`, and `constructor` segments in dotted config keys now uses inline literal comparisons at each property-write site (instead of a pre-loop `Set` check), so CodeQL's `js/prototype-pollution-utility` analysis recognises it as a sanitising barrier and code-scanning alert #26 clears. Runtime behaviour is unchanged from #663. Added regression tests that drive schema-valid dynamic-prefix keys (`agent_skills.__proto__`, `agent_skills.constructor`, `features.__proto__`, `review.models.constructor`) all the way to the guard — these reach `setConfigValue` past the schema gate and were previously the guard's only untested attack surface. (#751) diff --git a/.changeset/766-native-claude-plugin-manifest.md b/.changeset/766-native-claude-plugin-manifest.md deleted file mode 100644 index 9e4cdf9e7..000000000 --- a/.changeset/766-native-claude-plugin-manifest.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 797 ---- -**gsd-core can now be installed as a native Claude Code plugin** — a new `.claude-plugin/plugin.json` manifest enables installing gsd-core via `claude plugin install` or the zero-friction `~/.claude/skills/` auto-load path (`gsd-core@skills-dir`), with slash commands auto-namespaced as `/gsd-core:` (e.g. `/gsd-core:plan-phase`) and lifecycle management via `claude plugin enable|disable|update`. gsd-core's always-on guard and update hooks are wired for the plugin path through `hooks/hooks.json` using `${CLAUDE_PLUGIN_ROOT}`. This is additive — the existing npm / file-copy installer is unchanged. diff --git a/.changeset/768-claude-permissions-prepopulate.md b/.changeset/768-claude-permissions-prepopulate.md deleted file mode 100644 index 9aab9eaac..000000000 --- a/.changeset/768-claude-permissions-prepopulate.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 819 ---- -**Installer pre-populates `permissions.allow`/`deny` for Claude Code** — fresh Claude Code installs now receive GSD's known-safe tool-call patterns (`Bash(npx gsd-core *)`, `Read(.planning/*)`, `Write(.planning/*)`, `Read(STATE.md)`, `Write(STATE.md)`) in `settings.json` out of the box, eliminating first-run approval prompts. A `deny` block for credential files (`Read(.env)`, `Read(.env.*)`, `Read(.secrets)`) is also added for defense-in-depth. The merge is additive and idempotent; existing user-set entries are preserved. Uninstall removes only GSD-owned entries. (#768) diff --git a/.changeset/770-claude-lifecycle-hooks.md b/.changeset/770-claude-lifecycle-hooks.md deleted file mode 100644 index c69857eba..000000000 --- a/.changeset/770-claude-lifecycle-hooks.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -type: Added -pr: 821 ---- - -Added: register newly-available Claude Code lifecycle hooks — SubagentStop, Stop, PreCompact (all wired to gsd-context-monitor for context-headroom warnings), and FileChanged (matcher: `config.json`, wired to new gsd-config-reload.js hook that hot-reloads `.planning/config.json` context mid-session). Also updates hooks/hooks.json (plugin manifest) and managed-hooks-registry for drift-guard coverage (#770). diff --git a/.changeset/771-agent-named-colors.md b/.changeset/771-agent-named-colors.md deleted file mode 100644 index 6b8594ea5..000000000 --- a/.changeset/771-agent-named-colors.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Changed -pr: 823 ---- -Agent `color:` frontmatter now uses Claude Code's documented named colors (`red`/`blue`/`green`/`yellow`/`purple`/`orange`/`pink`/`cyan`) instead of hex values or the undocumented `magenta`, so the intended per-agent TUI color differentiation renders reliably across the Claude Code runtime. Display-only metadata; no behavior change. (#771) \ No newline at end of file diff --git a/.changeset/772-codex-hook-events-commandwindows.md b/.changeset/772-codex-hook-events-commandwindows.md deleted file mode 100644 index 78a942559..000000000 --- a/.changeset/772-codex-hook-events-commandwindows.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Changed -pr: 827 ---- -Codex installs now register three additional stable hook events (`SubagentStart`, `Stop`, `PostToolUse`) wired to `gsd-context-monitor.js`, matching the full event coverage available since Codex CLI stabilised these hooks. The `SessionStart` hook entry gains a `commandWindows` field on Windows installs so the `.cmd` shim is used for native execution (Git Bash/MSYS cannot POSIX-exec `node.exe` directly). Both new-event registration and uninstall paths handle the flat `{ "EventName": [...] }` and nested `{ "hooks": { "EventName": [...] } }` hooks.json shapes. `gsd-context-monitor.js` and its Windows `.cmd` sibling are added to the managed-hook allowlist so idempotent re-runs de-duplicate entries correctly. (#772) diff --git a/.changeset/774-codex-skill-metadata-service-tier.md b/.changeset/774-codex-skill-metadata-service-tier.md deleted file mode 100644 index 2f20d6218..000000000 --- a/.changeset/774-codex-skill-metadata-service-tier.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Changed -pr: 828 ---- -Codex CLI installs now emit two enrichments per agent and skill. **Agent TOML enrichment:** light-tier agents (haiku-equivalent, `routingTier: "light"` in model-catalog.json) get `service_tier = "flex"` and `model_verbosity = "low"` appended to their agent TOML, telling the Codex scheduler to use the flex tier (lower cost, background processing) and suppress verbose token output. **Skill TUI chip:** each installed `gsd-*` skill directory now receives an `agents/openai.yaml` file with `interface.display_name` and `interface.short_description`, making the skill appear in the Codex `/skills` picker with a human-readable name and description drawn from the skill's existing short-description frontmatter. Both enrichments are additive and backward-compatible with Codex CLI ≥ 0.130.0. (#774) diff --git a/.changeset/776-gemini-hook-events.md b/.changeset/776-gemini-hook-events.md deleted file mode 100644 index 7f03043df..000000000 --- a/.changeset/776-gemini-hook-events.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 829 ---- -Gemini installs now register three additional hook events — `BeforeAgent`, `AfterAgent`, and `BeforeModel` — wired to `gsd-context-monitor.js` for per-turn context headroom tracking. Previously only `SessionStart`, `BeforeTool`, and `AfterTool` were registered. The installer also detects `hooksConfig.enabled: false` in the user's Gemini `settings.json` and emits a clear warning, surfacing the silent failure mode where all hooks are registered but never execute. (#776) diff --git a/.changeset/778-cross-runtime-command-enrichment.md b/.changeset/778-cross-runtime-command-enrichment.md deleted file mode 100644 index 9ee123a24..000000000 --- a/.changeset/778-cross-runtime-command-enrichment.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 825 ---- -Cross-runtime command enrichment in the installer. Gemini CLI commands now use native `{{args}}` interpolation (translated from Claude's `$ARGUMENTS`) so typed arguments interpolate into the prompt body, and `/gsd:progress` injects live project state via a fixed, injection-safe `!{cat .planning/STATE.md 2>/dev/null}` shell block. Qwen Code skills now carry a numeric `priority` field so the most-used main-loop workflows (`new-project`, `plan-phase`, `execute-phase`, …) surface first in the `/skills` list. The OpenCode per-command `model`/`agent`/`subtask` enrichment was evaluated and intentionally not implemented — `model` would reintroduce the ProviderModelNotFoundError regression that the converter deliberately guards against for non-Anthropic providers (#1156), `subtask`/`agent` change execution semantics for GSD's interactive commands, and `variant` is not in the OpenCode command schema. (#778) diff --git a/.changeset/782-cline-skills-emission.md b/.changeset/782-cline-skills-emission.md deleted file mode 100644 index 076ac792b..000000000 --- a/.changeset/782-cline-skills-emission.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Changed -pr: 809 ---- -**Cline global installs now emit skills, not just rules:** gsd writes skills to `~/.cline/skills//SKILL.md` for Cline ≥ v3.48.0 (see [Cline skills docs](https://docs.cline.bot/customization/skills)), in addition to the existing `.clinerules` file. Each `SKILL.md` carries `name`/`description` frontmatter (agentskills.io) with paths rewritten to the `.cline/` convention. Local installs remain `.clinerules`-only. The `.clinerules` rules file continues to be emitted for compatibility, and upgrading over an existing rules-only install emits the new skills on the next run. diff --git a/.changeset/783-kilo-global-skills-base.md b/.changeset/783-kilo-global-skills-base.md deleted file mode 100644 index 5b439e4f0..000000000 --- a/.changeset/783-kilo-global-skills-base.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -type: Fixed -pr: 806 ---- -**`getGlobalSkillsBase('kilo')` now resolves to `~/.kilo/skills`** — where Kilo Code actually discovers global skills — instead of `~/.config/kilo/skills`. Per [Kilo Code docs](https://kilo.ai/docs/customize/skills), global skills live in the `.kilo` directory within HOME (`~/.kilo/skills/`), independent of the XDG-based config dir at `~/.config/kilo`. The kilo.jsonc config dir (`~/.config/kilo`) and the `command/` path used by the installer are correct and unchanged. Blast radius: this corrects the resolved skills-base path used by doctor/status checks and agent-skills-block resolution (`init.cjs`); the installer writes commands (not skills) for Kilo, so no files were previously being written to the wrong location. - - diff --git a/.changeset/784-opencode-kilo-skills.md b/.changeset/784-opencode-kilo-skills.md deleted file mode 100644 index 0d75bdfad..000000000 --- a/.changeset/784-opencode-kilo-skills.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 810 ---- -Emit native on-demand skills (`skills//SKILL.md`) for the OpenCode-family runtimes (OpenCode and Kilo) at install time, in addition to the existing flat `command/` and file-based `agents/` surfaces. OpenCode and Kilo share a config schema and both discover skills from `skills//SKILL.md`; the installer now stages each GSD command as a skill with minimal, spec-compliant frontmatter (`name` matching the directory, `description` 1–1024 chars) via a shared OpenCode-family skill writer. Skills respect the active install profile (core/minimal stage only their subset) and are removed on uninstall. (#784) diff --git a/.changeset/785-cursor-slash-commands.md b/.changeset/785-cursor-slash-commands.md deleted file mode 100644 index 62007389f..000000000 --- a/.changeset/785-cursor-slash-commands.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 803 ---- -`gsd install --cursor` now writes `.cursor/commands/gsd-.md` in addition to the existing `.cursor/skills/` surface. Cursor 1.6 introduced plain-markdown slash commands (no frontmatter) in `.cursor/commands/`; they appear in the `/` menu in the Agent input. Each command file is generated from the same source as the skill but with frontmatter stripped and Cursor-specific content transforms applied (`convertClaudeCommandToCursorCommand`). The skills surface is unchanged — both surfaces are written on every install. diff --git a/.changeset/786-copilot-hooks-agents.md b/.changeset/786-copilot-hooks-agents.md deleted file mode 100644 index 24271c046..000000000 --- a/.changeset/786-copilot-hooks-agents.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 804 ---- -The GitHub Copilot installer now reaches lifecycle-hook and instruction parity with other first-class runtimes. It emits a self-contained `sessionStart` hook config (`.github/hooks/gsd-session.json` for local installs, `~/.copilot/hooks/gsd-session.json` for global) and writes `AGENTS.md` at the repository root (which Copilot CLI reads as primary instructions) alongside `copilot-instructions.md`. The hook is an inline `command` hook with no separate script file, so it cannot dangle. Both artifacts are removed — with user-authored content preserved — on `--uninstall`. (#786) diff --git a/.changeset/787-cline-hooks-agents.md b/.changeset/787-cline-hooks-agents.md deleted file mode 100644 index f6eefd754..000000000 --- a/.changeset/787-cline-hooks-agents.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 803 ---- -Elevate the Cline runtime to hook parity. The installer now emits the Cline `.clinerules/` directory form (`.clinerules/gsd.md`) instead of a single `.clinerules` file, adds a `.clinerules/hooks/PreToolUse` lifecycle hook (Cline v3.36+ JSON stdin → `{cancel,errorMessage,contextModification}` protocol; guards `.planning/` artifacts and fails open), and merges GSD instructions into the cross-tool global `~/.agents/AGENTS.md` target on global installs. A legacy single-file `.clinerules` is migrated to the directory form in place, and `--uninstall` removes the new artifacts and strips the GSD block from `~/.agents/AGENTS.md`. (#787) diff --git a/.changeset/788-qwen-hook-events.md b/.changeset/788-qwen-hook-events.md deleted file mode 100644 index c04b19a68..000000000 --- a/.changeset/788-qwen-hook-events.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 807 ---- -Qwen Code installs now register three additional hook events that Qwen Code supports beyond Claude Code: `SubagentStop`, `Stop`, and `PreCompact` — all wired to `gsd-context-monitor.js` for context headroom tracking at subagent completion, model stop, and pre-compaction. These events are Qwen-only; Claude Code installs are unchanged. `UserPromptSubmit` is deferred: `gsd-prompt-guard` exits unless `tool_name` is `Write|Edit`, making it a no-op for that payload shape. (#788) diff --git a/.changeset/789-codebuddy-slash-commands.md b/.changeset/789-codebuddy-slash-commands.md deleted file mode 100644 index 244771897..000000000 --- a/.changeset/789-codebuddy-slash-commands.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -type: Added -pr: 830 ---- -**CodeBuddy (Tencent) installs now emit `/gsd-*` slash commands.** A `--codebuddy` install writes `commands/gsd-.md` files to `~/.codebuddy/commands/` so GSD workflows are invokable from CodeBuddy's `/` menu (`/gsd-phase`, `/gsd-ship`, etc.), matching the integration depth of other fully-elevated runtimes (#789). The existing `skills/gsd-/SKILL.md` files are now emitted with `user-invocable: false` so they stay out of the `/` menu — the commands surface is the single `/` entry point (no duplicate entries) and skills remain available for model invocation. Subagents (`~/.codebuddy/agents/`) were already emitted and are unchanged. Uninstall removes the `gsd-*` command files while preserving user-owned commands. No `mcp.json` is written — gsd ships no MCP server and CodeBuddy's `mcp.json` only registers external MCP servers. - - diff --git a/.changeset/790-augment-commands.md b/.changeset/790-augment-commands.md deleted file mode 100644 index 94e609ac0..000000000 --- a/.changeset/790-augment-commands.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -type: Added -pr: 801 ---- -**Augment (Auggie) installs now emit slash command definitions alongside skills.** A global `--augment` install writes `commands/gsd-.md` files to `~/.augment/commands/` in addition to the existing `skills/gsd-/SKILL.md` files, matching the integration depth of other fully-elevated runtimes and allowing Auggie users to invoke GSD as slash commands (`/gsd-phase`, `/gsd-ship`, etc.) without manual configuration (#790). Content rewrites (path normalisation and Augment-specific branding) are applied at install time. Uninstall removes the `gsd-*` command files while preserving user-owned commands. `mcpServers` registration is explicitly excluded — gsd ships no MCP server and does not register third-party servers. - - diff --git a/.changeset/812-copilot-home.md b/.changeset/812-copilot-home.md deleted file mode 100644 index 4a6cddc48..000000000 --- a/.changeset/812-copilot-home.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 814 ---- -Honor the `COPILOT_HOME` environment variable when resolving the GitHub Copilot global config directory. Previously a global `--copilot` install ignored `COPILOT_HOME` and wrote all artifacts (skills, agents, `copilot-instructions.md`, the session hook) to `~/.copilot` even when the user had relocated their Copilot home, making them undiscoverable by Copilot CLI. Resolution now follows `--config-dir` > `COPILOT_CONFIG_DIR` > `COPILOT_HOME` > `~/.copilot`, mirroring the existing `CODEX_HOME` handling. Uninstall uses the same resolver and stays symmetric. (#812) diff --git a/.changeset/clever-sloths-dart.md b/.changeset/clever-sloths-dart.md deleted file mode 100644 index 369373fb7..000000000 --- a/.changeset/clever-sloths-dart.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Changed -pr: 719 ---- -**Workflow size budget now measures bytes, not lines (#717).** `tests/workflow-size-budget.test.cjs` re-bases its tier ceilings (XL/LARGE/DEFAULT) from line counts to byte counts — deterministic, no tokenizer, and matching the unit vendors bound on (Codex's 32,768-byte project_doc_max_bytes cap). The #597 tighten-only ratchet and per-file semantics are unchanged; the budget's caching-independent quality rationale (context rot / attention budget) is now documented. diff --git a/.changeset/curious-seals-howl.md b/.changeset/curious-seals-howl.md deleted file mode 100644 index aee2587f9..000000000 --- a/.changeset/curious-seals-howl.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 845 ---- -**Release version bumps now keep runtime manifest versions in sync** — `.claude-plugin/plugin.json` and `gemini-extension.json` are stamped to match `package.json` on every `npm version`, unblocking RC/finalize releases. New version-bearing manifests must be registered in `scripts/sync-manifest-versions.cjs` (enforced by a regression test). diff --git a/.changeset/eager-birds-squeak.md b/.changeset/eager-birds-squeak.md deleted file mode 100644 index 1963ad041..000000000 --- a/.changeset/eager-birds-squeak.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 843 ---- -Issues are now checked for duplicates when opened: a no-LLM title-similarity check posts a challenge comment and applies a `possible-duplicate` label when a new issue closely matches existing open ones. Flagged issues that go unanswered for 24h are auto-closed as duplicates (reply, or react 👎 to the bot comment, to keep one open); a reply clears the label and routes to `needs-maintainer-review`. (#836) diff --git a/.changeset/eager-herons-forage.md b/.changeset/eager-herons-forage.md deleted file mode 100644 index f1adf4731..000000000 --- a/.changeset/eager-herons-forage.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 777 ---- -Cursor now receives GSD lifecycle hooks via `.cursor/hooks.json` — a sessionStart hook injects the current workflow state as context at session start, and a postToolUse hook nudges the agent to update `.planning/` after write-class operations, bringing Cursor to baseline hook parity with Gemini and Claude Code. diff --git a/.changeset/gemini-extension-package.md b/.changeset/gemini-extension-package.md deleted file mode 100644 index 5c527dc5a..000000000 --- a/.changeset/gemini-extension-package.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 775 ---- -**Gemini CLI extension package** — gsd-core now ships a `gemini-extension.json` manifest (plus a `GEMINI.md` context payload) at the repository root, so Gemini CLI users can install, update, and remove GSD through Gemini's own extension lifecycle: `gemini extensions install https://github.com/open-gsd/gsd-core`, `gemini extensions update gsd-core`, `gemini extensions uninstall gsd-core`, and `gemini extensions link ` for local dev. The extension is discoverable in `gemini extensions list` and loads GSD's operating context into every session. Additive — the existing `npx gsd-core --gemini` installer (which provides the `/gsd:*` slash commands) is unchanged. (#775) diff --git a/.changeset/gentle-orcas-click.md b/.changeset/gentle-orcas-click.md deleted file mode 100644 index e17689f51..000000000 --- a/.changeset/gentle-orcas-click.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Changed -pr: 753 ---- -The `gsd-verifier` agent no longer re-runs the full workspace test suite once per must-have during Step 7b spot-checks — it enumerates tests to prove existence and runs a single named test to prove a pass, invoking the full suite at most once per verification. diff --git a/.changeset/graceful-cranes-dart.md b/.changeset/graceful-cranes-dart.md deleted file mode 100644 index 279df7dab..000000000 --- a/.changeset/graceful-cranes-dart.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Changed -pr: 769 ---- -**`/gsd-plan-phase`, `/gsd-execute-phase`, `/gsd-autonomous` now run in an isolated forked context on Claude Code** — `context: fork` in skill frontmatter protects the main session's context budget. These three heavy skills also declare `effort: xhigh`; quick-status skills `/gsd-progress` and `/gsd-stats` declare `effort: low`. The installer preserves both fields when converting commands to Claude SKILL.md files. Runtimes that do not recognise these fields silently ignore them — no behaviour change on non-Claude runtimes. diff --git a/.changeset/humble-badgers-dart.md b/.changeset/humble-badgers-dart.md deleted file mode 100644 index ad1f67751..000000000 --- a/.changeset/humble-badgers-dart.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Security -pr: 665 ---- -**Hardened roadmap-phase parsing and config writes** — resolved ReDoS in phase-heading/plan-filename regexes (validate/verify/commands/phase), blocked prototype-pollution through dotted config keys in `config-set`, and pinned `qs >= 6.15.2` (DoS advisory). diff --git a/.changeset/humble-cats-rally.md b/.changeset/humble-cats-rally.md new file mode 100644 index 000000000..5b4bd97ff --- /dev/null +++ b/.changeset/humble-cats-rally.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1344 +--- +Init phase lookups now resolve active phases whose canonical details live in a flat Phase Details block outside the current milestone summary, restoring requirement coverage for plan/execute/phase-op flows. diff --git a/.changeset/humble-ibex-jump.md b/.changeset/humble-ibex-jump.md deleted file mode 100644 index 1454056cc..000000000 --- a/.changeset/humble-ibex-jump.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 675 ---- -**`npx @opengsd/gsd-core` upgrades no longer abort with "applied migration checksum changed"** — an already-applied installer migration whose recorded checksum drifted (e.g. a shipped body was edited) is now detected and reconciled automatically on the next install, instead of hard-failing the upgrade. Replaces the published-checksum allowlist with general self-healing recovery plus a CI baseline lock. diff --git a/.changeset/humble-tigers-tumble.md b/.changeset/humble-tigers-tumble.md deleted file mode 100644 index ebc2429a8..000000000 --- a/.changeset/humble-tigers-tumble.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 642 ---- -**`/gsd-import`, `/gsd-plan-review-convergence`, and `/gsd-spec-phase` now run on global installs** — these workflows resolve `gsd-tools` via the runtime launcher instead of a hardcoded `$HOME` path, so they no longer falsely report the tool as "not found" (and stop short) when only a global/shim install is present and no project-local runtime exists. diff --git a/.changeset/lazy-mvp-bodies-progressive-disclosure.md b/.changeset/lazy-mvp-bodies-progressive-disclosure.md deleted file mode 100644 index 2dad6c698..000000000 --- a/.changeset/lazy-mvp-bodies-progressive-disclosure.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Changed -pr: 746 ---- -**`/gsd:plan-phase` and `/gsd:execute-phase` no longer eagerly load MVP-only guidance on non-MVP runs** — the MVP planner rules, user-story template, Walking-Skeleton template, and MVP+TDD halt-report reference are now Read lazily by the planner/executor only when MVP / Walking-Skeleton / MVP+TDD mode is active, in both the workflow files and the `gsd-planner`/`gsd-executor` agent definitions, instead of being `@`-imported into every run. Behaviour is unchanged; non-MVP planning/execution simply carries less context. (#720) diff --git a/.changeset/mellow-yaks-bark.md b/.changeset/mellow-yaks-bark.md deleted file mode 100644 index 98227e60b..000000000 --- a/.changeset/mellow-yaks-bark.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -type: Changed -pr: 824 ---- - -Automated `codex exec` invocations in the review workflow now include `--ephemeral` (no session-state accumulation across automated/CI runs) and `--dangerously-bypass-hook-trust` (skip hook-trust prompts for hooks managed by gsd-core itself). These flags apply only to the non-interactive reviewer invocations in `gsd-core/workflows/review.md`. (#773) diff --git a/.changeset/nimble-dogs-purr.md b/.changeset/nimble-dogs-purr.md deleted file mode 100644 index 4a9233663..000000000 --- a/.changeset/nimble-dogs-purr.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 709 ---- -**Worktree wave-cleanup no longer fails when the phase SUMMARY is committed** — `rescueSummaryArtifacts` no longer copies an already-committed SUMMARY into the main checkout, which previously caused `git merge --no-ff` to abort with a permanent `merge_failed` (#706). diff --git a/.changeset/noble-cranes-sing.md b/.changeset/noble-cranes-sing.md deleted file mode 100644 index a032a87bb..000000000 --- a/.changeset/noble-cranes-sing.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 749 ---- -**Phase execution no longer halts with `exit 42` (worktree base mismatch) when run on a branch diverged from the default branch (#683).** Claude Code forks worktree-isolated executors off the repository default branch (`origin/HEAD`), so running `/gsd-execute-phase` on an unmerged milestone/feature branch left every executor without the phase's plan files and tripped the `worktree-branch-check` guard (100% reproducible, all OSes). Execute-phase now detects this before dispatch and automatically degrades to sequential execution on the main working tree, recommending the permanent fix `worktree.baseRef:"head"`. Both fresh installs and upgrades of GSD Core set `worktree.baseRef:"head"` in `.claude/settings.local.json` automatically (no-clobber) when `workflow.use_worktrees` is enabled (the default); `gsd-tools worktree set-baseref` remains available for manual use (e.g. after toggling worktrees on later). The `exit 42` guard remains as a backstop. diff --git a/.changeset/rapid-mice-bark.md b/.changeset/rapid-mice-bark.md deleted file mode 100644 index 7f08ec687..000000000 --- a/.changeset/rapid-mice-bark.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 754 ---- -**New `agent_skills_security.trusted_global_roots` config** — opt-in allowlist of trusted root directories so symlinked `global:` agent skills whose real path resolves outside the default skills dir (e.g. `~/.claude/skills`) are accepted; default `[]` is byte-identical and preserves the symlink-escape guard. diff --git a/.changeset/rapid-tigers-dance.md b/.changeset/rapid-tigers-dance.md deleted file mode 100644 index d4ee0b42a..000000000 --- a/.changeset/rapid-tigers-dance.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 710 ---- -**Codex install no longer corrupts launcher paths** — shell path segments like `${VAR}/gsd-core/` and `$(cmd)/gsd-local-patches` are no longer rewritten into a literal `$gsd-core` token during Codex markdown conversion (#704). diff --git a/.changeset/silly-zebras-forage.md b/.changeset/silly-zebras-forage.md deleted file mode 100644 index 7eabd459a..000000000 --- a/.changeset/silly-zebras-forage.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 817 ---- -**`/gsd:surface` no longer corrupts installed skill paths** — re-surfacing (profile/enable/disable/reset) now applies the same per-runtime path rewrites as install, so SKILL.md bodies keep the correct install target instead of reverting to the converter's default `~/.claude` paths. diff --git a/.changeset/sturdy-lemurs-roar.md b/.changeset/sturdy-lemurs-roar.md deleted file mode 100644 index a3a255f7b..000000000 --- a/.changeset/sturdy-lemurs-roar.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 707 ---- -**`/gsd:graphify`, `/gsd:import`, and planning agents now resolve `gsd-tools` on global/shim-only installs** — agent and command surfaces that invoked a hardcoded `$HOME/.claude/...gsd-tools.cjs` path now route through the resolved `gsd_run` launcher, so the step no longer reports the tool "not found" when there is no project-local runtime. diff --git a/.changeset/tidy-finches-jump.md b/.changeset/tidy-finches-jump.md deleted file mode 100644 index 531176f16..000000000 --- a/.changeset/tidy-finches-jump.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 822 ---- -**`/gsd:surface` no longer mis-names or orphans runtime command files** — re-surfacing now writes the same `gsd-`-prefixed command filenames as a fresh install for flat command dirs (Cursor, Augment, OpenCode, Kilo) and preserves user-authored command files instead of deleting them. diff --git a/.changeset/tidy-otters-glide.md b/.changeset/tidy-otters-glide.md deleted file mode 100644 index 700e9210b..000000000 --- a/.changeset/tidy-otters-glide.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Fixed -pr: 694 ---- -**`/gsd:update` reliably previews release notes again** — promotes the 1.3.x changelog into dated `[1.3.0]`/`[1.3.1]` sections, stops deleting the temp changelog before the human-readable render (no more `(changelog unavailable)`), and adds a release gate that blocks publishing a version whose `CHANGELOG.md` section was never promoted. diff --git a/.changeset/update-next-rc-channel.md b/.changeset/update-next-rc-channel.md deleted file mode 100644 index bfcbea65d..000000000 --- a/.changeset/update-next-rc-channel.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: Added -pr: 839 ---- -Added `/gsd-update --next` (alias `--rc`) to install or refresh from the `@next` RC dist-tag (ADR #660). A new `parse_update_channel` workflow step resolves the channel from `$ARGUMENTS`; the version check and all three npx install invocations thread `$TAG` instead of hardcoding `@latest`. When `--next` is used the version-comparison output gains a `Channel: next (RC)` banner so the user knows they are leaving the stable line; omitting the flag keeps `@latest` behavior byte-for-byte unchanged. `check-latest-version.cjs` gains `ALLOWED_TAGS`, `buildViewArgs`, and `resolveTag` exports, with an allowlist guard (enforced at both the CLI and function boundary) that rejects any dist-tag other than `latest`/`next`. (#815) diff --git a/.changeset/vivid-badgers-zip.md b/.changeset/vivid-badgers-zip.md deleted file mode 100644 index aa90b0e76..000000000 --- a/.changeset/vivid-badgers-zip.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -type: Changed -pr: 747 ---- -**Codex slash-command conversion no longer corrupts inline-wrapped `/gsd-…` file paths** — the install-time converter now identifies a real `/gsd-` mention by positive boundaries (opening delimiter + no path continuation) instead of an unbounded preceding-character denylist, closing the path-corruption class (#637 → #704) by construction while still converting legitimate backtick-wrapped mentions. - - diff --git a/.changeset/wire-changelog-render.md b/.changeset/wire-changelog-render.md deleted file mode 100644 index 1f6de5765..000000000 --- a/.changeset/wire-changelog-render.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -type: Changed -pr: 715 ---- -The release pipeline now automatically runs `changeset render` during the finalize job, promoting `.changeset/` fragments into a dated `CHANGELOG.md` section before publishing — previously a manual step that was routinely skipped (leaving v1.3.0 and v1.3.1 unpromoted, #690). A new `--allow-empty` flag prevents the verify gate from hard-failing on no-change releases by emitting a dated heading with a `_No notable changes._` placeholder when there are zero fragments. - diff --git a/.changeset/zesty-badgers-caper.md b/.changeset/zesty-badgers-caper.md new file mode 100644 index 000000000..2f911e84c --- /dev/null +++ b/.changeset/zesty-badgers-caper.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 1320 +--- +**`verify plan-structure` warns on cross-task region-scope conflicts (#968)** — when a plan task's file-wide negative grep (`! grep -Eq 'PAT' file` / `grep -c 'PAT' file == 0`) bans a construct a sibling task legitimately requires elsewhere in the same file, plan validation now surfaces a warning pointing to the new region/function-scoped negative-gate idiom (documented in the gsd-planner guidance and the planner-antipatterns reference, with a worked banned-in-X / required-in-Y example). Warn-only: it never errors and never changes `valid`. (#1320) diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index d45c84ffa..f2773efd1 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "gsd-core", "displayName": "GSD Core", - "version": "1.5.0-rc.4", + "version": "1.5.0-rc.5", "description": "GSD Core is a meta-prompting, context engineering, and spec-driven development system for AI coding agents.", "author": { "name": "open-gsd", diff --git a/.github/workflows/auto-backmerge.yml b/.github/workflows/auto-backmerge.yml index 110629122..a13361562 100644 --- a/.github/workflows/auto-backmerge.yml +++ b/.github/workflows/auto-backmerge.yml @@ -1,9 +1,17 @@ name: Auto Back-Merge main → next # Keep `next` at-or-ahead of `main` automatically. Every push to `main` -# (release finalize, hotfix finalize, occasional emergency fix) opens — or -# updates — a PR titled "chore: back-merge main → next". CI gates the merge; -# auto-merge is enabled so it lands as soon as it's green. +# (release finalize, hotfix finalize, occasional emergency fix) opens a PR +# titled "chore: back-merge main → next" and admin-merges it when it is safe. +# +# Resolution model (see #1336 / #1339): `next` is the integration branch and +# already contains every CODE change on `main` (fixes flow next -> release -> +# main). So the back-merge keeps `next`'s tree wholesale (`-s ours`, which never +# conflicts) and overlays only `main`'s release-engineering: the rendered +# CHANGELOG.md and the `.changeset` fragments `main` consumed at release. A +# safety net detects the rare case of a change that exists ONLY on `main` (an +# emergency fix pushed straight to `main`) and routes that PR to human review +# instead of auto-merging. # # Phase 1 of the next-branch migration: this workflow runs but is a no-op # (`if: false` on the job) until `next` exists in the repo. Once Phase 2 @@ -59,51 +67,57 @@ jobs: git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" - - name: Create or update back-merge branch + - name: Create back-merge branch (reconcile main → next) if: steps.check.outputs.next_exists == 'true' id: branch - env: - GH_TOKEN: ${{ secrets.GSD_BOT_PR_TOKEN || secrets.GITHUB_TOKEN }} run: | set -euo pipefail - SHORT_SHA=$(git rev-parse --short HEAD) + git fetch --no-tags origin main next + SHORT_SHA=$(git rev-parse --short origin/main) BR="chore/backmerge-main-to-next-${SHORT_SHA}" echo "branch=$BR" >> "$GITHUB_OUTPUT" - # If there's already an open back-merge PR, reuse its branch by - # updating it with the new main HEAD via a merge. Otherwise create - # a fresh branch. - EXISTING_PR=$(gh pr list --base next --label automation --label backmerge --state open \ - --json number,headRefName --jq '.[0]' 2>/dev/null || echo "") - EXISTING_BR=$(echo "$EXISTING_PR" | jq -r '.headRefName // empty') + git checkout -B "$BR" origin/next + BASE=$(git merge-base HEAD origin/main) - if [ -n "$EXISTING_BR" ]; then - case "$EXISTING_BR" in - chore/backmerge-main-to-next-*) ;; - *) echo "::error::refusing to operate on non-backmerge branch: $EXISTING_BR"; exit 1 ;; + # Keep next's tree wholesale and record main as a second parent so main + # becomes an ancestor of next (stops the back-merge backlog recurring). + # -s ours never conflicts; next already has main's code. + git merge -s ours --no-commit --no-ff origin/main + + # Overlay main's rendered CHANGELOG.md (the release promotions). + if git cat-file -e "origin/main:CHANGELOG.md" 2>/dev/null; then + git checkout origin/main -- CHANGELOG.md + git add CHANGELOG.md + fi + + # Replay main's .changeset changes vs the merge-base: prune fragments + # main consumed at release (D); bring any main-authored fragment (A/M). + while IFS="$(printf '\t')" read -r status file; do + [ -n "${file:-}" ] || continue + case "$status" in + D) git rm --quiet --ignore-unmatch -- "$file" ;; + A|M) git checkout origin/main -- "$file" && git add -- "$file" ;; esac - echo "Updating existing back-merge branch: $EXISTING_BR" - git fetch origin "$EXISTING_BR":"$EXISTING_BR" || true - git checkout "$EXISTING_BR" - # Fast-forward to current main if possible; otherwise merge. - git merge --no-edit origin/main || { - echo "::warning::Merge conflict back-merging main into existing back-merge branch. Human resolution required." - exit 1 - } - git push --force origin "$EXISTING_BR" - echo "branch=$EXISTING_BR" >> "$GITHUB_OUTPUT" - echo "reused=true" >> "$GITHUB_OUTPUT" + done < <(git diff --name-status "$BASE" origin/main -- .changeset) + + # Safety net: -s ours silently drops anything existing ONLY on main. + # List code files (excluding CHANGELOG/.changeset) that main changed + # since the merge-base but next did not — a rare straight-to-main fix. + MAIN_CODE=$(git diff --name-only "$BASE" origin/main -- . ':(exclude)CHANGELOG.md' ':(exclude).changeset' | sort) + NEXT_CODE=$(git diff --name-only "$BASE" origin/next -- . ':(exclude)CHANGELOG.md' ':(exclude).changeset' | sort) + DROPPED=$(comm -23 <(printf '%s\n' "$MAIN_CODE") <(printf '%s\n' "$NEXT_CODE") | grep -v '^$' || true) + + git commit -m "chore: back-merge main into next (${SHORT_SHA})" + git push --force origin "$BR" + + if [ -n "$DROPPED" ]; then + echo "needs_review=true" >> "$GITHUB_OUTPUT" + echo "dropped_oneline=$(printf '%s' "$DROPPED" | tr '\n' ' ')" >> "$GITHUB_OUTPUT" + echo "::warning::Back-merge auto-resolved in favor of next and dropped main-only change(s) in: $(printf '%s' "$DROPPED" | tr '\n' ' '). PR left open for manual review." else - git fetch origin next:next - git checkout -b "$BR" next - # Bring main's commits onto next via a merge commit (preserves tag history). - if ! git merge --no-edit -m "chore: back-merge main into next" origin/main; then - echo "::error::Cannot auto-back-merge main into next: merge conflict. A maintainer must back-merge manually (git checkout next; git merge origin/main; resolve; push)." - git merge --abort || true - exit 1 - fi - git push --force origin "$BR" - echo "reused=false" >> "$GITHUB_OUTPUT" + echo "needs_review=false" >> "$GITHUB_OUTPUT" + echo "dropped_oneline=" >> "$GITHUB_OUTPUT" fi - name: Sync next's version to main's released version @@ -115,10 +129,26 @@ jobs: [0-9]*.[0-9]*.[0-9]*) : ;; *) echo "refusing to sync next: unexpected version '$VERSION' from origin/main" >&2; exit 1 ;; esac - node scripts/sync-next-version.cjs "$VERSION" --in-place - git add -u - git diff --cached --quiet || git commit -m "chore: sync next package version to ${VERSION}" - git push origin HEAD + NEXT_VERSION=$(node -pe "require('./package.json').version") + # Never downgrade an ahead next (e.g. next on 1.5.0-rc.N while main is + # 1.4.5). Only sync when main's version is strictly higher; a release + # (no prerelease) outranks a prerelease at the same X.Y.Z base. + if node -e ' + const split = v => { const [b, pre] = String(v).split("-"); return [b.split(".").map(Number), pre]; }; + const [a, ap] = split(process.argv[1]); + const [b, bp] = split(process.argv[2]); + let c = 0; + for (let i = 0; i < 3; i++) { if (a[i] !== b[i]) { c = a[i] - b[i]; break; } } + if (c === 0 && !!ap !== !!bp) c = ap ? -1 : 1; // release > prerelease + process.exit(c > 0 ? 0 : 1); + ' "$VERSION" "$NEXT_VERSION"; then + node scripts/sync-next-version.cjs "$VERSION" --in-place + git add -u + git diff --cached --quiet || git commit -m "chore: sync next package version to ${VERSION}" + git push origin HEAD + else + echo "next ($NEXT_VERSION) is at or ahead of main ($VERSION); leaving next version unchanged." + fi - name: Open or update PR if: steps.check.outputs.next_exists == 'true' @@ -126,16 +156,24 @@ jobs: env: GH_TOKEN: ${{ secrets.GSD_BOT_PR_TOKEN || secrets.GITHUB_TOKEN }} BR: ${{ steps.branch.outputs.branch }} + NEEDS_REVIEW: ${{ steps.branch.outputs.needs_review }} + DROPPED_ONELINE: ${{ steps.branch.outputs.dropped_oneline }} run: | set -euo pipefail - SHORT_SHA=$(git rev-parse --short HEAD) + SHORT_SHA=$(git rev-parse --short origin/main) + REVIEW_NOTE="" + if [ "${NEEDS_REVIEW:-false}" = "true" ]; then + REVIEW_NOTE=" + REVIEW REQUIRED: this back-merge auto-resolved in favor of next and dropped main-only change(s) in: ${DROPPED_ONELINE}. A maintainer must verify these are not emergency fixes that need re-applying to next. Do not merge until verified." + fi BODY=$(cat <> "$GITHUB_OUTPUT" - name: Admin-merge the back-merge PR - if: steps.check.outputs.next_exists == 'true' + # Skip auto-merge when the back-merge dropped main-only changes — a human + # must review those before this lands. + if: steps.check.outputs.next_exists == 'true' && steps.branch.outputs.needs_review != 'true' env: GH_TOKEN: ${{ secrets.GSD_BOT_PR_TOKEN || secrets.GITHUB_TOKEN }} PR: ${{ steps.openpr.outputs.pr }} run: | - # Squash would lose the merge-commit context; use merge commit. + # Merge commit (not squash) preserves main as an ancestor of next. gh pr merge --admin --merge "$PR" diff --git a/.gitignore b/.gitignore index 97c9f8096..9a5f28709 100644 --- a/.gitignore +++ b/.gitignore @@ -186,6 +186,7 @@ build/ /gsd-core/bin/lib/workstream.cjs /gsd-core/bin/lib/roadmap.cjs /gsd-core/bin/lib/audit.cjs +/gsd-core/bin/lib/git-base-branch.cjs __pycache__/ *.pyc .venv/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 913a58ac8..72ed80a19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,125 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +## [1.4.5] - 2026-06-12 + +### Fixed + +- **`model_policy` is now honored on the default `claude` runtime** — including the `anthropic-fable` Claude Fable 5 preset. Policy-resolved model IDs map to Claude Code agent aliases (e.g. `claude-fable-5` → `fable`), and IDs without a Claude alias warn and fall back to the configured tier. Previously the entire `model_policy` block was silently ignored on `claude`. (#1133) (#1133) + +## [1.4.4] - 2026-06-11 + +### Changed + +- Added an opt-in `anthropic-fable` model policy provider preset for Claude Fable 5 high-budget routing while preserving the existing Anthropic Opus 4.8 defaults and `anthropic` provider preset. (#1014) (#1015) + +## [1.4.3] - 2026-06-09 + +### Fixed + +- Fix `--reapply` verifier false-positives on post-#604-rename installs caused by two gaps in pristine-baseline handling: + +**Gap 1** (`verify-reapply-patches.cjs`): when `backup-meta.json` records a `pristine_hash` for a file but `gsd-pristine/` has no corresponding snapshot on disk, the verifier fell to over-broad mode (every upstream-changed line treated as a user-added requirement) and produced `FAIL_USER_LINES_MISSING` false positives. Fix: return advisory `OK_NO_BASELINE` reason (non-blocking, exit 0) when a recorded hash is present but the pristine file is absent — the verifier cannot reason correctly without a baseline and must not block. + +**Gap 2** (new migration `004-prune-stale-pristine-snapshots`): migration 003 removed legacy `get-shit-done/` runtime files but left `gsd-pristine/get-shit-done/` orphan snapshots in place. Those stale snapshots referenced `get-shit-done/...` key paths that no longer match the active `gsd-core/...` layout, contributing to `FAIL_INSTALLED_MISSING` false reports. Fix: add a new migration (not editing 003, to preserve its checksum) that removes all files under `gsd-pristine/get-shit-done/`. (#934) (#937) +- **`/gsd-update` changelog preview no longer silently fails** — the installer now copies `scripts/changeset/` and `scripts/lib/` into the runtime config dir so `$GSD_DIR/scripts/changeset/cli.cjs` resolves at runtime; `update.md` was updated to use the correct installed path and to surface an explicit error if the CLI is missing rather than swallowing it. (#938) +- **`plan-review-convergence` now runs `gsd-plan-phase` inline instead of inside `Agent()`** — both sites that previously wrapped `gsd-plan-phase` in `Agent()` (initial planning + replan loop) have been changed to bare `Skill()` calls at depth 0. On Claude Code, a depth-1 Agent has no Agent tool, so a wrapped `plan-phase` could never spawn `gsd-planner` or `gsd-plan-checker` — the replan loop silently failed to produce a revised plan whenever HIGH concerns were found. Running plan-phase inline from the depth-0 orchestrator (which retains the Agent tool) restores the full planner→checker sub-agent chain. A new structural guard test (`bug-936-no-nested-spawner-wrap.test.cjs`) statically scans all workflow files and fails if any workflow wraps a spawner orchestrator in `Agent()` without a `RUNTIME != claude` carve-out, preventing regression. (#936) (#939) + +## [1.4.2] - 2026-06-09 + +### Fixed + +- **`/gsd-plan-phase`, `/gsd-execute-phase`, `/gsd-autonomous` no longer carry `context: fork`** — these are spawning orchestrators; a forked subagent context has no `Agent` tool, preventing them from spawning the subagents they require. `effort: xhigh` is preserved. Fixes `/gsd:autonomous` halting with "running as a forked subagent" on 1.4.1 (#921). Also replaces the introspection-based Agent-availability check in `plan-phase`'s `` block with an attempt-based gate: the workflow now always attempts the `Agent()` call and only stops if a real tool-unavailable error is returned, eliminating false-negative aborts in top-level sessions (#922). (#921) +- **`gsd-context-monitor.js` now echoes the actual invoking hook event name** — instead of hardcoding `hookEventName: "PostToolUse"` (or `"AfterTool"` for Gemini), the hook reads `data.hook_event_name` from the stdin payload and falls back to the runtime heuristic only when the field is absent or blank; this fixes Claude Code rejecting hook output with `"expected Stop but got PostToolUse"` when the monitor is invoked by the Stop, SubagentStop, or PreCompact hooks registered in PR #821. (#925) (#926) + +## [1.4.1] - 2026-06-09 + +### Changed + +- Added no-drift guard tests (`tests/issue-57-runtime-install-no-drift.test.cjs`) that protect the Runtime Install Policy Module boundary (ADR-58) and the explicit Runtime Config Adapter Registry (#60). They fail loudly when supported-runtime metadata is added to an installer call site (`allRuntimes`, the interactive `runtimeMap` menu) without a matching registry adapter entry, or when config-mutation dispatch escapes the registry's declared install surfaces — catching reintroduction of the scattered per-runtime branching those seams removed. (#867) + +### Fixed + +- **profile-pipeline temp output now lands under the reaped GSD temp root.** `cmdExtractMessages` and `cmdProfileSample` previously created their output directories directly in `os.tmpdir()` root (`gsd-pipeline-*` / `gsd-profile-*`), which `reapStaleTempFiles` never scans (it only scans `GSD_TEMP_DIR = os.tmpdir()/gsd`). The directories accumulated forever. Both sites now call `ensureGsdTempDir()` and create under `GSD_TEMP_DIR`. Also adds missing `after`/`afterEach` teardown to four test fixtures that leaked `gsd-*` temp dirs on every `npm test` run. (#866) (#879) +- **`gsd_run` launcher shim now probes all non-Claude runtime homes before failing.** The shim's last-resort detection previously stopped at `$HOME/.claude`, causing a false-positive fatal error on every non-Claude runtime (Hermes, Cursor, Codex, Copilot, Windsurf, Augment, Trae, Qwen, CodeBuddy, Cline, Grok, Antigravity, OpenCode, Kilo) when `RUNTIME_DIR` was unset and `gsd-tools` was not on `PATH`. The snippet now probes each runtime's config directory (respecting `HERMES_HOME`, `CURSOR_CONFIG_DIR`, `CODEX_HOME`, etc. with sensible `$HOME`-relative defaults) before emitting the install error. (#903) +- **`validate health` and `validate consistency` no longer emit false-positive W007 warnings for projects using checklist-style ROADMAP.md phases.** `buildRoadmapPhaseVariants()` in `src/validate.cts` previously used only a heading-style regex (`## Phase N: name`), silently ignoring the supported checklist format (`- [x] **Phase N: name**`). This caused every on-disk phase directory to trigger W007 ("exists on disk but not in ROADMAP.md") when the project's ROADMAP used checklist-only notation. The fix adds a second regex pass mirroring the existing `buildNotStartedPhaseVariants()` approach. Additionally, `cmdValidateConsistency()` in `src/verify.cts` had a duplicate inline heading-only regex with the same gap — refactored to delegate to `buildRoadmapPhaseVariants()` (DRY). (#892) (#893) +- **`init execute-phase` and `cmdCommit` now produce correct `branch_name` when `project_code` is set** — the `{phase}` substitution in `phase_branch_template` now calls `normalizePhaseName()`, stripping the project-code prefix and zero-padding the number, so the generated branch is e.g. `gsd/phase-01-foundation` instead of `gsd/phase-CK-01-foundation`. Both the execute-phase output path (`src/init.cts`) and the pre-execution commit path (`src/commands.cts`) are fixed. (#904) (#904) +- **`syncStateFrontmatter` no longer strips `current_phase`, `current_phase_name`, `current_plan`, and `progress` from `STATE.md`** — when body annotations are absent (e.g. after an agent rewrites the body), the existing frontmatter values for those scalars are now preserved, mirroring the fallback already applied in `cmdStateJson`. (#905) (#905) +- **Top-level Claude Code `/gsd-plan-phase` now always spawns the researcher/planner/plan-checker agents instead of collapsing them inline** — a `` block after `` makes the Agent-availability requirement explicit and documents that the workflow fails-closed (stops with a clear log message) in genuinely Agent-less contexts; seven "ORCHESTRATOR RULE — CODEX RUNTIME" labels are renamed to "ALL RUNTIMES" so the guard applies universally; `execute-phase.md` scopes its existing "Other runtimes" inline-fallback prose to non-Claude contexts, preserving the #853 backgrounded-agent behaviour. (#913) (#913) +- **`/gsd-manager` and `/gsd-autonomous --interactive` no longer silently skip worktree isolation and independent verification on Claude Code.** They dispatched plan/execute as background agents, but a backgrounded Claude Code agent has no Agent/Task tool and cannot spawn the nested executors, plan-checker, or verifier — so isolation and verification silently never ran. Both workflows now resolve the runtime and run plan/execute inline on Claude Code; background dispatch is kept on runtimes that support nested subagents. (#863) +- **Installer no longer leaks `gsd-cmd-rewrites-*` temp directories.** Each install that emitted slash commands left one `fs.mkdtempSync` directory under the system temp root; on `tmpfs` `/tmp` hosts these accumulated and consumed RAM-backed storage. `installRuntimeArtifacts()` now removes the temp copy in a `finally` once command files are copied. (#862) +- Corrected the installer `--help` profile skill counts: `core` now shows 8 (was 7) and `standard` shows 14 (was 13), both derived from `PROFILES` so they can't drift again; the `full` line drops the stale hardcoded `66` for `all skills`. (#834) (#847) + +## [1.4.0] - 2026-06-08 + +### Added + +- **Research is now cached, curated-first, and code-governed** — a content-addressed Research Store (per-source TTL), a single provider waterfall with confidence tiers, and registry-API package legitimacy replace the per-agent prose waterfall and the slopcheck bolt-on. (#664) Confidence is now verification-evidence-driven: provider identity alone no longer yields HIGH; HIGH requires ground-truth corroboration (e.g. `legitimacyVerdict: 'OK'`), authority alone caps at MEDIUM, and SLOP caps at LOW. (#664) +- `/gsd:plan-phase` now accepts a `--granularity ` flag to override the configured planning granularity for a single invocation. The flag takes precedence over `granularities.planning`, top-level `granularity`, and `planning.granularity` config. Invalid values are rejected. (#703) (#750) +- **gsd-core can now be installed as a native Claude Code plugin** — a new `.claude-plugin/plugin.json` manifest enables installing gsd-core via `claude plugin install` or the zero-friction `~/.claude/skills/` auto-load path (`gsd-core@skills-dir`), with slash commands auto-namespaced as `/gsd-core:` (e.g. `/gsd-core:plan-phase`) and lifecycle management via `claude plugin enable|disable|update`. gsd-core's always-on guard and update hooks are wired for the plugin path through `hooks/hooks.json` using `${CLAUDE_PLUGIN_ROOT}`. This is additive — the existing npm / file-copy installer is unchanged. (#797) +- **Installer pre-populates `permissions.allow`/`deny` for Claude Code** — fresh Claude Code installs now receive GSD's known-safe tool-call patterns (`Bash(npx gsd-core *)`, `Read(.planning/*)`, `Write(.planning/*)`, `Read(STATE.md)`, `Write(STATE.md)`) in `settings.json` out of the box, eliminating first-run approval prompts. A `deny` block for credential files (`Read(.env)`, `Read(.env.*)`, `Read(.secrets)`) is also added for defense-in-depth. The merge is additive and idempotent; existing user-set entries are preserved. Uninstall removes only GSD-owned entries. (#768) (#819) +- +Added: register newly-available Claude Code lifecycle hooks — SubagentStop, Stop, PreCompact (all wired to gsd-context-monitor for context-headroom warnings), and FileChanged (matcher: `config.json`, wired to new gsd-config-reload.js hook that hot-reloads `.planning/config.json` context mid-session). Also updates hooks/hooks.json (plugin manifest) and managed-hooks-registry for drift-guard coverage (#770). (#821) +- Gemini installs now register three additional hook events — `BeforeAgent`, `AfterAgent`, and `BeforeModel` — wired to `gsd-context-monitor.js` for per-turn context headroom tracking. Previously only `SessionStart`, `BeforeTool`, and `AfterTool` were registered. The installer also detects `hooksConfig.enabled: false` in the user's Gemini `settings.json` and emits a clear warning, surfacing the silent failure mode where all hooks are registered but never execute. (#776) (#829) +- Cross-runtime command enrichment in the installer. Gemini CLI commands now use native `{{args}}` interpolation (translated from Claude's `$ARGUMENTS`) so typed arguments interpolate into the prompt body, and `/gsd:progress` injects live project state via a fixed, injection-safe `!{cat .planning/STATE.md 2>/dev/null}` shell block. Qwen Code skills now carry a numeric `priority` field so the most-used main-loop workflows (`new-project`, `plan-phase`, `execute-phase`, …) surface first in the `/skills` list. The OpenCode per-command `model`/`agent`/`subtask` enrichment was evaluated and intentionally not implemented — `model` would reintroduce the ProviderModelNotFoundError regression that the converter deliberately guards against for non-Anthropic providers (#1156), `subtask`/`agent` change execution semantics for GSD's interactive commands, and `variant` is not in the OpenCode command schema. (#778) (#825) +- Emit native on-demand skills (`skills//SKILL.md`) for the OpenCode-family runtimes (OpenCode and Kilo) at install time, in addition to the existing flat `command/` and file-based `agents/` surfaces. OpenCode and Kilo share a config schema and both discover skills from `skills//SKILL.md`; the installer now stages each GSD command as a skill with minimal, spec-compliant frontmatter (`name` matching the directory, `description` 1–1024 chars) via a shared OpenCode-family skill writer. Skills respect the active install profile (core/minimal stage only their subset) and are removed on uninstall. (#784) (#810) +- `gsd install --cursor` now writes `.cursor/commands/gsd-.md` in addition to the existing `.cursor/skills/` surface. Cursor 1.6 introduced plain-markdown slash commands (no frontmatter) in `.cursor/commands/`; they appear in the `/` menu in the Agent input. Each command file is generated from the same source as the skill but with frontmatter stripped and Cursor-specific content transforms applied (`convertClaudeCommandToCursorCommand`). The skills surface is unchanged — both surfaces are written on every install. (#803) +- The GitHub Copilot installer now reaches lifecycle-hook and instruction parity with other first-class runtimes. It emits a self-contained `sessionStart` hook config (`.github/hooks/gsd-session.json` for local installs, `~/.copilot/hooks/gsd-session.json` for global) and writes `AGENTS.md` at the repository root (which Copilot CLI reads as primary instructions) alongside `copilot-instructions.md`. The hook is an inline `command` hook with no separate script file, so it cannot dangle. Both artifacts are removed — with user-authored content preserved — on `--uninstall`. (#786) (#804) +- Elevate the Cline runtime to hook parity. The installer now emits the Cline `.clinerules/` directory form (`.clinerules/gsd.md`) instead of a single `.clinerules` file, adds a `.clinerules/hooks/PreToolUse` lifecycle hook (Cline v3.36+ JSON stdin → `{cancel,errorMessage,contextModification}` protocol; guards `.planning/` artifacts and fails open), and merges GSD instructions into the cross-tool global `~/.agents/AGENTS.md` target on global installs. A legacy single-file `.clinerules` is migrated to the directory form in place, and `--uninstall` removes the new artifacts and strips the GSD block from `~/.agents/AGENTS.md`. (#787) (#803) +- Qwen Code installs now register three additional hook events that Qwen Code supports beyond Claude Code: `SubagentStop`, `Stop`, and `PreCompact` — all wired to `gsd-context-monitor.js` for context headroom tracking at subagent completion, model stop, and pre-compaction. These events are Qwen-only; Claude Code installs are unchanged. `UserPromptSubmit` is deferred: `gsd-prompt-guard` exits unless `tool_name` is `Write|Edit`, making it a no-op for that payload shape. (#788) (#807) +- **CodeBuddy (Tencent) installs now emit `/gsd-*` slash commands.** A `--codebuddy` install writes `commands/gsd-.md` files to `~/.codebuddy/commands/` so GSD workflows are invokable from CodeBuddy's `/` menu (`/gsd-phase`, `/gsd-ship`, etc.), matching the integration depth of other fully-elevated runtimes (#789). The existing `skills/gsd-/SKILL.md` files are now emitted with `user-invocable: false` so they stay out of the `/` menu — the commands surface is the single `/` entry point (no duplicate entries) and skills remain available for model invocation. Subagents (`~/.codebuddy/agents/`) were already emitted and are unchanged. Uninstall removes the `gsd-*` command files while preserving user-owned commands. No `mcp.json` is written — gsd ships no MCP server and CodeBuddy's `mcp.json` only registers external MCP servers. + + (#830) +- **Augment installs now emit slash command definitions alongside skills.** A global `--augment` install writes `commands/gsd-.md` files to `~/.augment/commands/` in addition to the existing `skills/gsd-/SKILL.md` files, matching the integration depth of other fully-elevated runtimes and allowing Auggie users to invoke GSD as slash commands (`/gsd-phase`, `/gsd-ship`, etc.) without manual configuration (#790). Content rewrites (path normalisation and Augment-specific branding) are applied at install time. Uninstall removes the `gsd-*` command files while preserving user-owned commands. `mcpServers` registration is explicitly excluded — gsd ships no MCP server and does not register third-party servers. (#801) +- Issues are now checked for duplicates when opened: a no-LLM title-similarity check posts a challenge comment and applies a `possible-duplicate` label when a new issue closely matches existing open ones. Flagged issues that go unanswered for 24h are auto-closed as duplicates (reply, or react 👎 to the bot comment, to keep one open); a reply clears the label and routes to `needs-maintainer-review`. (#836) (#843) +- Cursor now receives GSD lifecycle hooks via `.cursor/hooks.json` — a sessionStart hook injects the current workflow state as context at session start, and a postToolUse hook nudges the agent to update `.planning/` after write-class operations, bringing Cursor to baseline hook parity with Gemini and Claude Code. (#777) +- **Gemini CLI extension package** — gsd-core now ships a `gemini-extension.json` manifest (plus a `GEMINI.md` context payload) at the repository root, so Gemini CLI users can install, update, and remove GSD through Gemini's own extension lifecycle: `gemini extensions install https://github.com/open-gsd/gsd-core`, `gemini extensions update gsd-core`, `gemini extensions uninstall gsd-core`, and `gemini extensions link ` for local dev. The extension is discoverable in `gemini extensions list` and loads GSD's operating context into every session. Additive — the existing `npx gsd-core --gemini` installer (which provides the `/gsd:*` slash commands) is unchanged. (#775) (#775) +- **New `agent_skills_security.trusted_global_roots` config** — opt-in allowlist of trusted root directories so symlinked `global:` agent skills whose real path resolves outside the default skills dir (e.g. `~/.claude/skills`) are accepted; default `[]` is byte-identical and preserves the symlink-escape guard. (#754) +- Added `/gsd-update --next` (alias `--rc`) to install or refresh from the `@next` RC dist-tag (ADR #660). A new `parse_update_channel` workflow step resolves the channel from `$ARGUMENTS`; the version check and all three npx install invocations thread `$TAG` instead of hardcoding `@latest`. When `--next` is used the version-comparison output gains a `Channel: next (RC)` banner so the user knows they are leaving the stable line; omitting the flag keeps `@latest` behavior byte-for-byte unchanged. `check-latest-version.cjs` gains `ALLOWED_TAGS`, `buildViewArgs`, and `resolveTag` exports, with an allowlist guard (enforced at both the CLI and function boundary) that rejects any dist-tag other than `latest`/`next`. (#815) (#839) + +### Changed + +- `/gsd:plan-phase --research-phase ` now auto-uses an existing `RESEARCH.md` instead of prompting update/view/skip. When research already exists and neither `--research` nor `--view` is passed, it emits a one-line notice and exits cleanly, matching the promptless behavior of standard `/gsd:plan-phase `. Pass `--research` to force-refresh or `--view` to print the existing research. (#159) (#718) +- Retire the installer's one-off runtime directory helpers (`getGlobalDir`/`getOpencodeGlobalDir`/`getKiloGlobalDir`) and consolidate per-runtime global config-dir resolution onto the single canonical projection `runtime-homes:getGlobalConfigDir`, extended with the `--config-dir` override and the opencode/kilo `*_CONFIG` file-path precedence. Behavior-preserving across all 15 install runtimes. (#56) (#802) +- Make per-runtime config-mutation dispatch in the installer explicit: a new runtime config adapter registry maps each supported runtime to a typed config intent (install surface, shared-settings gate, finish-phase permission writer), and `install()`/`finishInstall()` dispatch by resolved intent instead of inline `runtime === '...'` branching. Behavior-preserving; unknown runtimes now fail loudly. (#60) (#795) +- **Verification status routing is now owned by a single queryable seam** — `ship.md` and `execute-phase.md` both consume `gsd_run query verification.status` instead of re-deriving the `passed`/`gaps_found`/`human_needed` routing independently; the query returns `next_action` and `next_command` so per-status prose no longer needs to be kept in sync across files. This also fixes the broad-grep status misread in `execute-phase.md` where a body `status:` line (in a code block or copied artifact) could concatenate with the frontmatter value and misroute a valid passed phase; a parity test fails if a new verifier status value lacks a route. (#651) (#755) +- Agent `color:` frontmatter now uses Claude Code's documented named colors (`red`/`blue`/`green`/`yellow`/`purple`/`orange`/`pink`/`cyan`) instead of hex values or the undocumented `magenta`, so the intended per-agent TUI color differentiation renders reliably across the Claude Code runtime. Display-only metadata; no behavior change. (#771) (#823) +- Codex installs now register three additional stable hook events (`SubagentStart`, `Stop`, `PostToolUse`) wired to `gsd-context-monitor.js`, matching the full event coverage available since Codex CLI stabilised these hooks. The `SessionStart` hook entry gains a `commandWindows` field on Windows installs so the `.cmd` shim is used for native execution (Git Bash/MSYS cannot POSIX-exec `node.exe` directly). Both new-event registration and uninstall paths handle the flat `{ "EventName": [...] }` and nested `{ "hooks": { "EventName": [...] } }` hooks.json shapes. `gsd-context-monitor.js` and its Windows `.cmd` sibling are added to the managed-hook allowlist so idempotent re-runs de-duplicate entries correctly. (#772) (#827) +- Codex CLI installs now emit two enrichments per agent and skill. **Agent TOML enrichment:** light-tier agents (haiku-equivalent, `routingTier: "light"` in model-catalog.json) get `service_tier = "flex"` and `model_verbosity = "low"` appended to their agent TOML, telling the Codex scheduler to use the flex tier (lower cost, background processing) and suppress verbose token output. **Skill TUI chip:** each installed `gsd-*` skill directory now receives an `agents/openai.yaml` file with `interface.display_name` and `interface.short_description`, making the skill appear in the Codex `/skills` picker with a human-readable name and description drawn from the skill's existing short-description frontmatter. Both enrichments are additive and backward-compatible with Codex CLI ≥ 0.130.0. (#774) (#828) +- **Cline global installs now emit skills, not just rules:** gsd writes skills to `~/.cline/skills//SKILL.md` for Cline ≥ v3.48.0 (see [Cline skills docs](https://docs.cline.bot/customization/skills)), in addition to the existing `.clinerules` file. Each `SKILL.md` carries `name`/`description` frontmatter (agentskills.io) with paths rewritten to the `.cline/` convention. Local installs remain `.clinerules`-only. The `.clinerules` rules file continues to be emitted for compatibility, and upgrading over an existing rules-only install emits the new skills on the next run. (#809) +- **Workflow size budget now measures bytes, not lines (#717).** `tests/workflow-size-budget.test.cjs` re-bases its tier ceilings (XL/LARGE/DEFAULT) from line counts to byte counts — deterministic, no tokenizer, and matching the unit vendors bound on (Codex's 32,768-byte project_doc_max_bytes cap). The #597 tighten-only ratchet and per-file semantics are unchanged; the budget's caching-independent quality rationale (context rot / attention budget) is now documented. (#719) +- The `gsd-verifier` agent no longer re-runs the full workspace test suite once per must-have during Step 7b spot-checks — it enumerates tests to prove existence and runs a single named test to prove a pass, invoking the full suite at most once per verification. (#753) +- **`/gsd-plan-phase`, `/gsd-execute-phase`, `/gsd-autonomous` now run in an isolated forked context on Claude Code** — `context: fork` in skill frontmatter protects the main session's context budget. These three heavy skills also declare `effort: xhigh`; quick-status skills `/gsd-progress` and `/gsd-stats` declare `effort: low`. The installer preserves both fields when converting commands to Claude SKILL.md files. Runtimes that do not recognise these fields silently ignore them — no behaviour change on non-Claude runtimes. (#769) +- **`/gsd:plan-phase` and `/gsd:execute-phase` no longer eagerly load MVP-only guidance on non-MVP runs** — the MVP planner rules, user-story template, Walking-Skeleton template, and MVP+TDD halt-report reference are now Read lazily by the planner/executor only when MVP / Walking-Skeleton / MVP+TDD mode is active, in both the workflow files and the `gsd-planner`/`gsd-executor` agent definitions, instead of being `@`-imported into every run. Behaviour is unchanged; non-MVP planning/execution simply carries less context. (#720) (#746) +- +Automated `codex exec` invocations in the review workflow now include `--ephemeral` (no session-state accumulation across automated/CI runs) and `--dangerously-bypass-hook-trust` (skip hook-trust prompts for hooks managed by gsd-core itself). These flags apply only to the non-interactive reviewer invocations in `gsd-core/workflows/review.md`. (#773) (#824) +- **Codex slash-command conversion no longer corrupts inline-wrapped `/gsd-…` file paths** — the install-time converter now identifies a real `/gsd-` mention by positive boundaries (opening delimiter + no path continuation) instead of an unbounded preceding-character denylist, closing the path-corruption class (#637 → #704) by construction while still converting legitimate backtick-wrapped mentions. (#747) +- The release pipeline now automatically runs `changeset render` during the finalize job, promoting `.changeset/` fragments into a dated `CHANGELOG.md` section before publishing — previously a manual step that was routinely skipped (leaving v1.3.0 and v1.3.1 unpromoted, #690). A new `--allow-empty` flag prevents the verify gate from hard-failing on no-change releases by emitting a dated heading with a `_No notable changes._` placeholder when there are zero fragments. (#715) + +### Fixed + +- `/gsd-review --cursor` now actually invokes the Cursor agent. Detection probes the `cursor-agent` headless binary instead of the `cursor` IDE launcher, the invocation calls the single `cursor-agent` binary in print mode (not the two-token `cursor agent`, which the IDE treats as a file path), and the review prompt is passed as a file-path argument rather than piped to stdin (which `cursor-agent -p` ignores). On failure the captured stderr is surfaced instead of a silent empty result. (#686) +- **No more "gsd-core" console-window flash on Windows.** Every gsd-core child process now passes `windowsHide: true`: the context monitor's `record-session` spawn, the `execGit` / `execNpm` / `execTool` helpers in `shell-command-projection`, the `gsd-worktree-path-guard` and `gsd-workflow-guard` hook git probes, `check-command-router`'s `git log` call, and the `roadmap-upgrade` git status/rev-parse/reset/clean calls — matching the existing `gsd-check-update` spawn. `execNpm` (which uses `shell: true` → `cmd.exe` and runs on every SessionStart, i.e. every `/clear`) and the worktree-path guard (which runs on every Edit/Write in a worktree) were the most visible offenders. No behavior change on macOS/Linux, where the flag is ignored. (#688) +- **`/gsd-review --agy` no longer hangs the whole review on large prompts.** On a big, file-path-rich prompt Antigravity's `agy -p` agentic Cascade can loop on its `code_search`/grep steps and never converge. The invocation now passes agy's own `--print-timeout` flag (its native print-mode cap) so a stalled run self-terminates through the tool's own mechanism; on a non-zero exit any partial output is discarded so the existing transcript fallback / "review failed" stub take over. (#689) +- The roadmap parser now resolves fresh phases of the current milestone in multi-milestone roadmaps. `extractCurrentMilestone()` scoped the current-milestone window to its `## Phases` checklist subsection and stopped at the milestone's own `## Milestone … (Phase Details)` heading, so the `### Phase N:` detail headers fell out of scope. Any command backed by the parser — `init.phase-op` (and therefore `/gsd:discuss-phase` and `/gsd:plan-phase`), `state`, `roadmap list`, and `validate health` (W006) — could not resolve phases of any milestone after the first until a `.planning/phases/` directory already existed, blocking discuss/plan. The parser now also includes the current milestone's `(Phase Details)` section in scope, anchored to the selected milestone's version token so sibling sub-milestones do not cross-pollinate. (#730) (#748) +- **`getGlobalSkillsBase('kilo')` now resolves to `~/.kilo/skills`** — where Kilo Code actually discovers global skills — instead of `~/.config/kilo/skills`. Per [Kilo Code docs](https://kilo.ai/docs/customize/skills), global skills live in the `.kilo` directory within HOME (`~/.kilo/skills/`), independent of the XDG-based config dir at `~/.config/kilo`. The kilo.jsonc config dir (`~/.config/kilo`) and the `command/` path used by the installer are correct and unchanged. Blast radius: this corrects the resolved skills-base path used by doctor/status checks and agent-skills-block resolution (`init.cjs`); the installer writes commands (not skills) for Kilo, so no files were previously being written to the wrong location. (#806) +- Honor the `COPILOT_HOME` environment variable when resolving the GitHub Copilot global config directory. Previously a global `--copilot` install ignored `COPILOT_HOME` and wrote all artifacts (skills, agents, `copilot-instructions.md`, the session hook) to `~/.copilot` even when the user had relocated their Copilot home, making them undiscoverable by Copilot CLI. Resolution now follows `--config-dir` > `COPILOT_CONFIG_DIR` > `COPILOT_HOME` > `~/.copilot`, mirroring the existing `CODEX_HOME` handling. Uninstall uses the same resolver and stays symmetric. (#812) (#814) +- **Release version bumps now keep runtime manifest versions in sync** — `.claude-plugin/plugin.json` and `gemini-extension.json` are stamped to match `package.json` on every `npm version`, unblocking RC/finalize releases. New version-bearing manifests must be registered in `scripts/sync-manifest-versions.cjs` (enforced by a regression test). (#845) +- **`npx @opengsd/gsd-core` upgrades no longer abort with "applied migration checksum changed"** — an already-applied installer migration whose recorded checksum drifted (e.g. a shipped body was edited) is now detected and reconciled automatically on the next install, instead of hard-failing the upgrade. Replaces the published-checksum allowlist with general self-healing recovery plus a CI baseline lock. (#675) +- **`/gsd-import`, `/gsd-plan-review-convergence`, and `/gsd-spec-phase` now run on global installs** — these workflows resolve `gsd-tools` via the runtime launcher instead of a hardcoded `$HOME` path, so they no longer falsely report the tool as "not found" (and stop short) when only a global/shim install is present and no project-local runtime exists. (#642) +- **Worktree wave-cleanup no longer fails when the phase SUMMARY is committed** — `rescueSummaryArtifacts` no longer copies an already-committed SUMMARY into the main checkout, which previously caused `git merge --no-ff` to abort with a permanent `merge_failed` (#706). (#709) +- **Phase execution no longer halts with `exit 42` (worktree base mismatch) when run on a branch diverged from the default branch (#683).** Claude Code forks worktree-isolated executors off the repository default branch (`origin/HEAD`), so running `/gsd-execute-phase` on an unmerged milestone/feature branch left every executor without the phase's plan files and tripped the `worktree-branch-check` guard (100% reproducible, all OSes). Execute-phase now detects this before dispatch and automatically degrades to sequential execution on the main working tree, recommending the permanent fix `worktree.baseRef:"head"`. Both fresh installs and upgrades of GSD Core set `worktree.baseRef:"head"` in `.claude/settings.local.json` automatically (no-clobber) when `workflow.use_worktrees` is enabled (the default); `gsd-tools worktree set-baseref` remains available for manual use (e.g. after toggling worktrees on later). The `exit 42` guard remains as a backstop. (#749) +- **Codex install no longer corrupts launcher paths** — shell path segments like `${VAR}/gsd-core/` and `$(cmd)/gsd-local-patches` are no longer rewritten into a literal `$gsd-core` token during Codex markdown conversion (#704). (#710) +- **`/gsd:surface` no longer corrupts installed skill paths** — re-surfacing (profile/enable/disable/reset) now applies the same per-runtime path rewrites as install, so SKILL.md bodies keep the correct install target instead of reverting to the converter's default `~/.claude` paths. (#817) +- **`/gsd:graphify`, `/gsd:import`, and planning agents now resolve `gsd-tools` on global/shim-only installs** — agent and command surfaces that invoked a hardcoded `$HOME/.claude/...gsd-tools.cjs` path now route through the resolved `gsd_run` launcher, so the step no longer reports the tool "not found" when there is no project-local runtime. (#707) +- **`/gsd:surface` no longer mis-names or orphans runtime command files** — re-surfacing now writes the same `gsd-`-prefixed command filenames as a fresh install for flat command dirs (Cursor, Augment, OpenCode, Kilo) and preserves user-authored command files instead of deleting them. (#822) +- **`/gsd:update` reliably previews release notes again** — promotes the 1.3.x changelog into dated `[1.3.0]`/`[1.3.1]` sections, stops deleting the temp changelog before the human-readable render (no more `(changelog unavailable)`), and adds a release gate that blocks publishing a version whose `CHANGELOG.md` section was never promoted. (#694) + +### Security + +- **`gsd-tools config-set` prototype-pollution guard hardened and regression-tested.** The guard that blocks `__proto__`, `prototype`, and `constructor` segments in dotted config keys now uses inline literal comparisons at each property-write site (instead of a pre-loop `Set` check), so CodeQL's `js/prototype-pollution-utility` analysis recognises it as a sanitising barrier and code-scanning alert #26 clears. Runtime behaviour is unchanged from #663. Added regression tests that drive schema-valid dynamic-prefix keys (`agent_skills.__proto__`, `agent_skills.constructor`, `features.__proto__`, `review.models.constructor`) all the way to the guard — these reach `setConfigValue` past the schema gate and were previously the guard's only untested attack surface. (#751) (#752) +- **Hardened roadmap-phase parsing and config writes** — resolved ReDoS in phase-heading/plan-filename regexes (validate/verify/commands/phase), blocked prototype-pollution through dotted config keys in `config-set`, and pinned `qs >= 6.15.2` (DoS advisory). (#665) + ## [1.3.1](https://www.npmjs.com/package/@opengsd/gsd-core/v/1.3.1) - 2026-06-04 ### Security diff --git a/CONTEXT.md b/CONTEXT.md index 4078f1bb2..fbb349075 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -170,7 +170,7 @@ ADR-857 phase 3b seam that merges capability-declared config slices into the `lo A named, stable site on a host loop step (per-step `pre`/`post` plus per-wave in Execute; 12 total) where Capabilities register hooks. Three hook kinds: `step` (runs as its own sequenced unit), `contribution` (injects into the core step's prompt/context), and `gate` (checks and optionally blocks via a declared `blocking` flag). Each hook declares the artifacts it produces and consumes; hook order is derived by topological sort of that produces/consumes graph (capability-id tiebreak), which also defines data flow — file-artifact based, surviving `/clear` and fresh executor contexts. Hooks are surfaced by runtime resolution with concrete projection: the workflow calls a query that resolves the active hooks and returns fully-rendered, ordered markdown for the executor. Failure is default-resilient — a non-gate hook that errors is skipped with a warning; a hook may opt into `onError: halt`. Part of the Capability system. ADR-857 phase 3c ships the registry-consuming query layer: `gsd-core/bin/lib/loop-resolver.cjs` exposes `resolveLoopHooks({ point, registry, config })` (pure, no I/O), `renderLoopHooks(resolved)` (pure markdown renderer), and `cmdLoopRenderHooks(cwd, point, raw, opts)` (I/O entry point); activated via `gsd-tools loop render-hooks ` which emits `{ point, activeHooks[], rendered }`. Activation is driven by `when` (dotted config key resolved against `loadConfig`), with inline literal `__proto__`/`constructor`/`prototype` prototype-pollution guard. The first phase-6 cutovers wiring workflows to this query have landed — ui-phase at `plan:pre` and ui-review at `verify:post` (in `plan-phase.md`/`autonomous.md`); further per-feature cutovers are ongoing. ### Capability State Resolver -ADR-857 phase 4b/6 unified resolver that composes the three toggle systems (install profile, runtime surface, config activation) into one per-capability view consumed by workflow hook rendering. Source of truth: `gsd-core/bin/lib/capability-state.cjs` (generated from `src/capability-state.cts`). Interface: `resolveCapabilityState({ registry, installedSkills, surfacedSkills, config, cwd? }) → { capabilities: CapabilityStateEntry[] }` (pure, no I/O); `resolveCapabilityRuntimeState(cwd, runtimeConfigDir)` (I/O resolver shared by workflow dispatch and diagnostics); `cmdCapabilityState(cwd, runtimeConfigDir, raw, opts)` (I/O output entry point); `isCapabilityActive(capId, cwd): boolean` (convenience predicate — calls `resolveCapabilityRuntimeState`, finds the entry, returns `entry.active`; `false` when capability not found). CLI surface: `gsd-tools capability state [--config-dir ]` — emits `{ runtimeConfigDir, capabilities[] }`. Per-capability output: `{ id, tier, skills[], installed, surfaced, enabled, active, hooks[] }` where `installed` = every owned skill ∈ installedSkills (or `installedSkills==='*'`; vacuously true for empty-skills caps), `surfaced` = every owned skill ∈ surfacedSkills (vacuously true for empty-skills caps), `enabled = installed && surfaced` (unchanged — install+surface toggle only), `active = enabled && configActivation` (tri-state deepening: configActivation resolves the capability's `activationKey` via `_resolveActivationValue`; absent `activationKey` → configActivation=true, so `active===enabled` for ungated capabilities), and `hooks` = `[{ point, kind: 'step'|'gate'|'contribution', when, configured, active }]` derived from the cap's `steps`, `gates`, `contributions` arrays (`configured` resolves `when`; `active = enabled && configured`). Capabilities sorted by `id` for determinism. Defensive: malformed registry → `{ capabilities: [] }`, never throws; inline literal `__proto__`/`constructor`/`prototype` prototype-pollution guard on capability id keys. +ADR-857 phase 4b/6 unified resolver that composes the three toggle systems (install profile, runtime surface, config activation) into one per-capability view consumed by workflow hook rendering. Source of truth: `gsd-core/bin/lib/capability-state.cjs` (generated from `src/capability-state.cts`). Interface: `resolveCapabilityState({ registry, installedSkills, surfacedSkills, config, cwd? }) → { capabilities: CapabilityStateEntry[] }` (pure, no I/O); `resolveCapabilityRuntimeState(cwd, runtimeConfigDir)` (I/O resolver shared by workflow dispatch and diagnostics — returns `{ runtimeConfigDir, warnings, capabilities }` only; `registry` and `config` are NOT returned — callers that need them import `capability-registry.cjs` and call `loadConfig(cwd)` directly); `cmdCapabilityState(cwd, runtimeConfigDir, raw, opts)` (I/O output entry point); `isCapabilityActive(capId, cwd): boolean` (convenience predicate — calls `resolveCapabilityRuntimeState`, finds the entry, returns `entry.active`; `false` when capability not found). CLI surface: `gsd-tools capability state [--config-dir ]` — emits `{ runtimeConfigDir, capabilities[] }`. Per-capability output: `{ id, tier, skills[], installed, surfaced, enabled, active, hooks[] }` where `installed` = every owned skill ∈ installedSkills (or `installedSkills==='*'`; vacuously true for empty-skills caps), `surfaced` = every owned skill ∈ surfacedSkills (vacuously true for empty-skills caps), `enabled = installed && surfaced` (unchanged — install+surface toggle only), `active = enabled && configActivation` (tri-state deepening: configActivation resolves the capability's `activationKey` via `_resolveActivationValue`; absent `activationKey` → configActivation=true, so `active===enabled` for ungated capabilities), and `hooks` = `[{ point, kind: 'step'|'gate'|'contribution', when, configured, active }]` derived from the cap's `steps`, `gates`, `contributions` arrays (`configured` resolves `when`; `active = enabled && configured`). Capabilities sorted by `id` for determinism. Defensive: malformed registry → `{ capabilities: [] }`, never throws; inline literal `__proto__`/`constructor`/`prototype` prototype-pollution guard on capability id keys. ### Capability State Writer The write-side mirror of the Capability State Resolver. Takes a desired capability state — per-capability `enabled` plus per-hook `gates` — and projects it onto the substrates: `enabled` drives the runtime surface (`.gsd-surface.json`) as the capability on/off switch; `gates` drive the federated config keys (`config.json` `workflow.*`) for hook-level granularity; the install profile (`.gsd-profile`) is a read-only floor it never writes. Writes the surface once and config once (atomic per substrate), then re-runs the resolver and reports divergence (assert-and-report) — so 'off means off' holds as a write-time invariant rather than by caller discipline. Source of truth: `src/capability-writer.cts`; the surface and config writers become its internal adapters. @@ -190,6 +190,8 @@ A Capability whose integration shape brings its own external process, service, o `RULESET.CAPABILITY.step-additive-gate-blocks=a `step` hook is purely additive (invoke skill + produce artifacts, NEVER halts the host); host-blocking preconditions are `gate`s (blocking:true, onError:halt); runtime/mode context (auto/chain vs manual) self-gates IN THE SKILL, not via `when` (config-only). §5.6 = plan:pre step (ui-phase; skill self-gates on frontend+pipeline, auto-fires only in pipelines) + a NEW plan:pre gate (frontend-and-no-UI-SPEC → halt, when:workflow.ui_safety_gate); the loop.render-hooks dispatch template handles steps AND gates. Resolves #1022.` +`RULESET.CAPABILITY.precedence-engine-single-owner=the config-key four-level precedence walk (loadConfig result → workstream config.json → root config.json → registry.configSchema default → absent) is owned solely by src/capability-activation.cts: raw-value primitive resolveConfigKey(dotKey, {config,cwd,registry}) and boolean wrapper _resolveActivationValue(dotKey,config,cwd,registry); loop-resolver.cts imports the engine (no duplicate); resolveConfigValues in loop-resolver.cts delegates to resolveConfigKey; resolveCapabilityRuntimeState does NOT return registry/config — callers import capability-registry.cjs and call loadConfig(cwd) directly.` + ### Wing A MemPalace organizational unit corresponding to one project or repository. GSD derives the wing name from `project_code` or the project directory when `mempalace.wing` is unset. A wing contains Rooms. Cross-project Tunnels connect rooms across wings. MemPalace vocabulary — see Connected Capability, MemPalace memory capability (issue #956). diff --git a/LICENSE b/LICENSE index 332687536..d47f8ea21 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ MIT License -Copyright (c) 2025 Lex Christopherson +Copyright (c) 2026 Open GSD Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/agents/gsd-planner.md b/agents/gsd-planner.md index 89717878a..cdc7e4cb9 100644 --- a/agents/gsd-planner.md +++ b/agents/gsd-planner.md @@ -198,6 +198,10 @@ Every task has four required fields: Full rules + worked examples: @gsd-core/references/planner-antipatterns.md ("Comment-Text Discipline"). + +**Region-scoped negative gates (WARN, #968):** Region-scope a file-wide negative grep when a sibling task needs that construct elsewhere in the same file; `validate_plan` WARNS. See: @gsd-core/references/planner-antipatterns.md ("Region-Scoped Negative Gates"). + + **:** Acceptance criteria - measurable state of completion. - Good: "Valid credentials return 200 + JWT cookie, invalid credentials return 401" - Bad: "Authentication is complete" diff --git a/docs/reference/plan-md.md b/docs/reference/plan-md.md index e2de09fbc..38982e2d8 100644 --- a/docs/reference/plan-md.md +++ b/docs/reference/plan-md.md @@ -187,7 +187,7 @@ Plans that contain any checkpoint task must set `autonomous: false` in frontmatt | `` | Files the executor must read before touching anything — the file being modified, any source-of-truth pattern file, any file whose types or conventions must be replicated. | | `` | Concrete instructions with exact identifiers, file paths, function signatures, and expected values. Never says "align X with Y" without specifying the target state. Never contains fenced code blocks or full implementations. | | `` | A runnable command or check that proves the task succeeded. Must distinguish pass from fail — `echo "done"` is not valid. | -| `` | Verifiable conditions: grep-verifiable strings, command exit codes, observable behaviours. No subjective language ("looks correct", "properly configured"). | +| `` | Verifiable conditions: grep-verifiable strings, command exit codes, observable behaviours. No subjective language ("looks correct", "properly configured"). Negative greps (`! grep -Eq 'PAT' file`) are file-scoped — region-scope them (`sed -n`/`awk` range, then grep) when a sibling task needs the construct elsewhere in the same file (#968). | | `` | A short measurable statement of the completed outcome. | --- diff --git a/gemini-extension.json b/gemini-extension.json index 6e2e76691..83db10363 100644 --- a/gemini-extension.json +++ b/gemini-extension.json @@ -1,6 +1,6 @@ { "name": "gsd-core", - "version": "1.5.0-rc.4", + "version": "1.5.0-rc.5", "description": "GSD Core — a meta-prompting, context engineering, and spec-driven development system for AI coding agents. Loads gsd's operating context into every Gemini CLI session.", "contextFileName": "GEMINI.md" } diff --git a/gsd-core/bin/lib/git-base-branch.cjs b/gsd-core/bin/lib/git-base-branch.cjs deleted file mode 100644 index b30cfda93..000000000 --- a/gsd-core/bin/lib/git-base-branch.cjs +++ /dev/null @@ -1,220 +0,0 @@ -"use strict"; -/** - * Git Base-Branch Resolver — issue #1146. - * - * Single source of truth for detecting the repository's default branch. - * Replaces the duplicated per-workflow bash detection that only consulted - * `refs/remotes/origin/HEAD` then hardcoded `:-main`, which silently - * returned "main" for repos whose default branch is "master" whenever - * origin/HEAD was unset (git init + remote add / fetch without set-head / - * most CI checkouts / many worktrees). - * - * Precedence ladder (highest to lowest): - * 1. `git.base_branch` config override from .planning/config.json - * 2. `git symbolic-ref --short refs/remotes/origin/HEAD` (fast, no network) - * 3. `git remote show origin` HEAD branch ← AUTHORITATIVE; works when #2 unset - * 4. Local branch existence: "master" present + "main" absent → "master"; - * "main" present → "main" - * 5. "main" (last-resort default) - * - * Every git subprocess is bounded with a timeout (≤ 30 s); on timeout/error - * the resolver degrades gracefully to the next tier — it never throws. - * - * Pure/testable: all I/O is injectable via the `deps` argument so unit - * tests can run without touching the real filesystem or spawning real git. - */ -var __importDefault = (this && this.__importDefault) || function (mod) { - return (mod && mod.__esModule) ? mod : { "default": mod }; -}; -Object.defineProperty(exports, "__esModule", { value: true }); -exports.readConfigBaseBranch = readConfigBaseBranch; -exports.trySymbolicRef = trySymbolicRef; -exports.tryRemoteShow = tryRemoteShow; -exports.tryLocalBranch = tryLocalBranch; -exports.resolveBaseBranch = resolveBaseBranch; -exports.gitWorktreeInfoInternal = gitWorktreeInfoInternal; -exports.cmdGitBaseBranch = cmdGitBaseBranch; -const node_fs_1 = __importDefault(require("node:fs")); -const node_path_1 = __importDefault(require("node:path")); -const shell_command_projection_cjs_1 = require("./shell-command-projection.cjs"); -// ─── Helpers ────────────────────────────────────────────────────────────────── -/** - * Safely look up `git.base_branch` from the project's config.json. - * Returns the configured value (a non-empty, non-null string) or null. - */ -function readConfigBaseBranch(planningDir, deps) { - const readFile = deps?.readFile ?? - ((p) => { try { - return node_fs_1.default.readFileSync(p, 'utf8'); - } - catch { - return null; - } }); - const configPath = node_path_1.default.join(planningDir, 'config.json'); - const raw = readFile(configPath); - if (!raw) - return null; - let cfg; - try { - cfg = JSON.parse(raw); - } - catch { - return null; - } - if (!cfg || typeof cfg !== 'object' || Array.isArray(cfg)) - return null; - const top = cfg; - // Support both "git.base_branch" (nested) and "base_branch" (flat legacy) - const gitSection = top.git; - if (gitSection && typeof gitSection === 'object' && !Array.isArray(gitSection)) { - const nested = gitSection.base_branch; - if (typeof nested === 'string' && nested.trim()) - return nested.trim(); - } - const flat = top.base_branch; - if (typeof flat === 'string' && flat.trim()) - return flat.trim(); - return null; -} -/** - * Try `git symbolic-ref --short refs/remotes/origin/HEAD` (no network). - * Strips the `origin/` prefix to return just the branch name. - * Returns null if unset or on error/timeout. - */ -function trySymbolicRef(cwd, execGit) { - try { - const r = execGit(['symbolic-ref', '--quiet', '--short', 'refs/remotes/origin/HEAD'], { cwd, timeout: 5_000 }); - if (r.exitCode !== 0 || !r.stdout) - return null; - // Output is e.g. "origin/main" — strip the prefix - const branch = r.stdout.trim().replace(/^origin\//, ''); - return branch || null; - } - catch { - return null; - } -} -/** - * Try `git remote show origin` to read the HEAD branch. - * This is authoritative when origin/HEAD is unset locally. - * Requires network access but succeeds in the common CI case where - * origin/HEAD was never set after `git init && git remote add origin`. - * - * Parses the line: `HEAD branch: ` - * Returns null on error, timeout, or if the output is malformed. - */ -function tryRemoteShow(cwd, execGit) { - try { - const r = execGit(['remote', 'show', 'origin'], { cwd, timeout: 15_000 }); - if (r.exitCode !== 0 || !r.stdout) - return null; - // The line looks like: " HEAD branch: master" - const m = r.stdout.match(/^\s*HEAD branch:\s*(\S+)\s*$/m); - if (!m) - return null; - const branch = m[1]; - // git emits "(unknown)" when the remote is offline but the local cache - // resolved it; treat that as non-authoritative and fall through. - if (!branch || branch === '(unknown)') - return null; - return branch; - } - catch { - return null; - } -} -/** - * Detect local branch existence as a tie-breaker when no remote info is available. - * - * Rules: - * - "master" present AND "main" absent → "master" - * - "main" present → "main" - * - Neither → null (fall through to default) - * - * Returns null on error/timeout. - */ -function tryLocalBranch(cwd, execGit) { - try { - const r = execGit(['branch', '--list', 'main', 'master'], { cwd, timeout: 5_000 }); - if (r.exitCode !== 0 || !r.stdout) - return null; - // `git branch --list main master` outputs one line per matching branch - const lines = r.stdout.split('\n').map(l => l.trim().replace(/^\*\s*/, '')); - const hasMain = lines.includes('main'); - const hasMaster = lines.includes('master'); - if (hasMaster && !hasMain) - return 'master'; - if (hasMain) - return 'main'; - return null; - } - catch { - return null; - } -} -/** - * Resolve the default/base branch for the repository at `cwd`. - * - * Consults the full precedence ladder and always returns a non-empty string. - * Never throws. - */ -function resolveBaseBranch(cwd, deps) { - const execGit = deps?.execGit ?? shell_command_projection_cjs_1.execGit; - // Derive .planning dir relative to cwd (mirrors planningDir() in core.cjs) - const planningDir = node_path_1.default.join(cwd, '.planning'); - // 1. Config override - const configured = readConfigBaseBranch(planningDir, deps); - if (configured) - return configured; - // 2. symbolic-ref (fast, no network) - const symref = trySymbolicRef(cwd, execGit); - if (symref) - return symref; - // 3. git remote show origin (authoritative when origin/HEAD unset) - const remoteShow = tryRemoteShow(cwd, execGit); - if (remoteShow) - return remoteShow; - // 4. Local branch existence - const local = tryLocalBranch(cwd, execGit); - if (local) - return local; - // 5. Last-resort default - return 'main'; -} -/** - * Detect whether `cwd` sits inside a git worktree, and if so, return the - * absolute path of the worktree root. - */ -function gitWorktreeInfoInternal(cwd) { - try { - const insideResult = (0, shell_command_projection_cjs_1.execGit)(['rev-parse', '--is-inside-work-tree'], { cwd, timeout: 5000 }); - if (insideResult.exitCode !== 0) { - return { inside: false, worktreeRoot: null }; - } - const insideStdout = String(insideResult.stdout || '').trim(); - if (insideStdout !== 'true') { - return { inside: false, worktreeRoot: null }; - } - const rootResult = (0, shell_command_projection_cjs_1.execGit)(['rev-parse', '--show-toplevel'], { cwd, timeout: 5000 }); - if (rootResult.exitCode !== 0) { - return { inside: true, worktreeRoot: null }; - } - const root = String(rootResult.stdout || '').trim(); - return { inside: true, worktreeRoot: root || null }; - } - catch { - return { inside: false, worktreeRoot: null }; - } -} -// ─── CLI entry point ────────────────────────────────────────────────────────── -/** - * CLI command: `gsd-tools git base-branch` - * Resolves the default branch and writes it to stdout (raw string, newline-terminated). - * Called by workflows via `gsd_run query git.base-branch`. - */ -function cmdGitBaseBranch(cwd, _args, deps) { - const branch = resolveBaseBranch(cwd, deps); - const write = deps?.write ?? ((s) => process.stdout.write(s)); - write(branch + '\n'); - return branch; -} diff --git a/gsd-core/references/planner-antipatterns.md b/gsd-core/references/planner-antipatterns.md index 1005c5cce..26e5d1fad 100644 --- a/gsd-core/references/planner-antipatterns.md +++ b/gsd-core/references/planner-antipatterns.md @@ -128,3 +128,49 @@ When the literal MUST appear in the plan body verbatim — e.g. the plan documen ``` One marker per literal. The marker exempts only the exact literal it names. + +## Region-Scoped Negative Gates + +> Surfaced at plan-write time by `verify.plan-structure` (the `validate_plan` step), WARN-level. Issue #968. + +A **negative grep** — `! grep -Eq 'PAT' file` or `grep -c 'PAT' file == 0` — asserts a construct is absent. `grep` is file-scoped by nature: it has no notion of function or region. This breaks down when a phase splits one file across parallel tasks with legitimately opposite needs for the same construct in different regions: + +- **Task A** bans the construct file-wide — a synchronous factory must not block on a refresh: `! grep -Eq 'await .*refresh' app/page.py`. +- **Task B** legitimately requires it elsewhere in the same file — a post-reindex handler must `await bridge.refresh()` to repopulate state. + +Both occurrences are real, correct production code in different functions of one file. A file-wide negative grep cannot say "absent in function X, present in function Y", so the two gates are mutually unsatisfiable with a direct call — the executor is pushed into an indirection whose only purpose is to relocate the matched string out of the file (pure gate-appeasement, zero behaviour change). This is distinct from Comment-Text Discipline (#429): there is no comment echo and no allowlist helps — the construct must genuinely be present in one region and absent in another. + +**The fix:** region-scope the negative gate so "absent in region X" stops implying "absent file-wide." + +### Bad — file-wide ban unsatisfiable against a sibling's real code + +```xml + +! grep -Eq 'await .*refresh' app/page.py + +Add a post-reindex handler in app/page.py that awaits bridge.refresh(). +app/page.py +``` + +Task B writing `await bridge.refresh()` trips Task A's file-wide gate, though both are correct. + +### Good — scope the gate to the factory region + +```xml + +! awk '/^def make_page/,/^def /' app/page.py | grep -Eq 'await .*refresh' + +! sed -n '12,40p' app/page.py | grep -Eq 'await .*refresh' +``` + +The factory region is asserted clean; the reindex handler elsewhere in the same file keeps its required `await bridge.refresh()`. Both gates pass with no code restructuring. Prefer an AST/structural check or a focused unit test where region extraction is fragile. + +### When the split is intentional and unavoidable + +If region-scoping is genuinely impractical and the file split is intentional, suppress the warning with a marker naming the pattern: + +``` + +``` + +One marker per pattern. The marker exempts only the exact pattern it names. Prefer region-scoping over suppression. diff --git a/next-branch-files.tar.gz b/next-branch-files.tar.gz deleted file mode 100644 index 6d4a90728..000000000 Binary files a/next-branch-files.tar.gz and /dev/null differ diff --git a/package-lock.json b/package-lock.json index 75296076f..2826d75fa 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@opengsd/gsd-core", - "version": "1.5.0-rc.4", + "version": "1.5.0-rc.5", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@opengsd/gsd-core", - "version": "1.5.0-rc.4", + "version": "1.5.0-rc.5", "license": "MIT", "dependencies": { "@anthropic-ai/claude-agent-sdk": "^0.2.84", @@ -27,7 +27,7 @@ "eslint-plugin-no-only-tests": "^3.4.0", "fast-check": "^4.8.0", "globals": "^16.5.0", - "js-yaml": "^4.1.1", + "js-yaml": "^4.2.0", "typescript": "^6.0.3", "typescript-eslint": "^8.60.0" }, @@ -3559,9 +3559,9 @@ } }, "node_modules/hono": { - "version": "4.12.23", - "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.23.tgz", - "integrity": "sha512-eIaZ9qDgu7XV0pxOCrg7/WhnQ6Ivm22UcxhXx/A3dcbqbbYgBEkc6e/J/s7j2tS96zoB0S9VBdLwQNCWwUo4LA==", + "version": "4.12.25", + "resolved": "https://registry.npmjs.org/hono/-/hono-4.12.25.tgz", + "integrity": "sha512-2NFaIyNVgJmBs/ecmtGzlmluTFs5cHEWGTdu0t1HBwYzoGXOL5nUQBRMXsXWla5i4KkG//QMzVP88m1+I3fdAQ==", "license": "MIT", "engines": { "node": ">=16.9.0" @@ -3828,10 +3828,20 @@ "license": "MIT" }, "node_modules/js-yaml": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.1.tgz", - "integrity": "sha512-qQKT4zQxXl8lLwBtHMWwaTcGfFOZviOJet3Oy/xmGk2gZH677CJM9EvtfdSkgWcATZhj/55JZ0rmy3myCT5lsA==", + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.2.0.tgz", + "integrity": "sha512-ePWsvanv0DWuDRsW8dnt+R4jQ31SCRCQ7hhNcPXZPsoBZiemuZNYGf7adZdqX2D86j6rvKp3RpCxVTSb8WQlOw==", "dev": true, + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/puzrin" + }, + { + "type": "github", + "url": "https://github.com/sponsors/nodeca" + } + ], "license": "MIT", "dependencies": { "argparse": "^2.0.1" diff --git a/package.json b/package.json index 9a647afef..9337cef27 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@opengsd/gsd-core", - "version": "1.5.0-rc.4", + "version": "1.5.0-rc.5", "description": "GSD Core is a meta-prompting, context engineering, and spec-driven development system for AI coding agents.", "bin": { "gsd-core": "bin/install.js", @@ -62,7 +62,7 @@ "eslint-plugin-no-only-tests": "^3.4.0", "fast-check": "^4.8.0", "globals": "^16.5.0", - "js-yaml": "^4.1.1", + "js-yaml": "^4.2.0", "typescript": "^6.0.3", "typescript-eslint": "^8.60.0" }, diff --git a/rollout-next-phase1.sh b/rollout-next-phase1.sh deleted file mode 100755 index 338ca4585..000000000 --- a/rollout-next-phase1.sh +++ /dev/null @@ -1,546 +0,0 @@ -#!/usr/bin/env zsh -# rollout-next-phase1.sh — native zsh, runs under macOS Terminal's default shell. -# DO NOT prefix with `bash`. Run as: ./rollout-next-phase1.sh -# -# Phase 1 of the `next` integration-branch rollout. -# -# What it does (in order, on YOUR Mac, in your repo dir): -# 1. Stashes any uncommitted work on your current branch (codex/...) with a -# named stash you can recover with `git stash list` + `git stash pop`. -# 2. Switches to main, pulls --ff-only to be sure you're current. -# 3. Creates branch: chore/introduce-next-integration-branch -# 4. Unpacks the 5 new files from the tarball. -# 5. Applies the 2 edits (branch-naming.yml, CONTRIBUTING.md) programmatically -# against the clean origin/main versions — no chance of pulling in -# unrelated commits from your current codex/ branch. -# 6. Sanity-checks YAML + bash syntax. -# 7. Commits with a conventional-commit message. -# 8. Pushes the branch to origin. -# 9. Files a `type: chore` issue using gh. -# 10. Renames the ADR file XXXX-...md → -...md and replaces XXXX in -# the ADR body with the real issue number. -# 11. Amends the commit, force-pushes-with-lease. -# 12. Opens the PR against main with `Closes #` in the body. -# -# Idempotent: re-running after a failure converges. Each step checks if it's -# already done. -# -# Usage: -# cd /Volumes/Mini\ Me/Users/trekkie/projects/gsd-core -# bash /path/to/rollout-next-phase1.sh -# -# Env overrides: -# TARBALL=/path/to/next-branch-files.tar.gz (default: ./next-branch-files.tar.gz) -# REPO=open-gsd/gsd-core (default: that) -# DRY_RUN=1 (skip push, issue, PR) - -set -euo pipefail - -# ─────────────────────────────────────────────────────────── -# Config -# ─────────────────────────────────────────────────────────── -REPO="${REPO:-open-gsd/gsd-core}" -TARBALL="${TARBALL:-./next-branch-files.tar.gz}" -BRANCH="chore/introduce-next-integration-branch" -DRY_RUN="${DRY_RUN:-0}" - -# Colors for readability (no-op if not a tty) -if [ -t 1 ]; then - C_BOLD=$'\033[1m'; C_GRN=$'\033[32m'; C_YEL=$'\033[33m'; C_RED=$'\033[31m'; C_DIM=$'\033[2m'; C_RST=$'\033[0m' -else - C_BOLD=''; C_GRN=''; C_YEL=''; C_RED=''; C_DIM=''; C_RST='' -fi - -step() { echo; echo "${C_BOLD}▸ $*${C_RST}"; } -ok() { echo "${C_GRN} ✓${C_RST} $*"; } -warn() { echo "${C_YEL} ⚠${C_RST} $*"; } -die() { echo "${C_RED} ✗ $*${C_RST}" >&2; exit 1; } -note() { echo "${C_DIM} $*${C_RST}"; } - -# ─────────────────────────────────────────────────────────── -# Step 0: Sanity checks -# ─────────────────────────────────────────────────────────── -step "Sanity checks" - -[ -d .git ] || die "Not in a git repo. cd to your gsd-core checkout first." -command -v gh >/dev/null || die "gh CLI not found. Install from https://cli.github.com/" -command -v jq >/dev/null || die "jq not found. Install: brew install jq" -gh auth status >/dev/null 2>&1 || die "gh not authenticated. Run: gh auth login" -[ -f "$TARBALL" ] || die "Tarball not found at: $TARBALL (override with TARBALL=/path/to/next-branch-files.tar.gz)" - -# Verify we're pointed at the right remote. -REMOTE_URL=$(git remote get-url origin 2>/dev/null || true) -case "$REMOTE_URL" in - *"$REPO"*) ok "Remote: $REMOTE_URL" ;; - *) die "Remote 'origin' is $REMOTE_URL — expected to contain $REPO. Wrong checkout?" ;; -esac - -# Verify tarball contents look right (fail loudly if user grabbed the wrong tar). -EXPECTED_PATHS=( - "docs/branching.md" - "docs/adr/XXXX-introduce-next-integration-branch.md" - ".github/workflows/auto-backmerge.yml" - ".github/workflows/pr-target-validator.yml" - "scripts/setup-branch-protection.sh" -) -TARLIST=$(tar tzf "$TARBALL") -for p in "${EXPECTED_PATHS[@]}"; do - echo "$TARLIST" | grep -qx "$p" || die "Tarball missing expected file: $p" -done -ok "Tarball contains all 5 expected files" - -# ─────────────────────────────────────────────────────────── -# Step 1: Stash any uncommitted work (protecting rollout files from the sweep) -# ─────────────────────────────────────────────────────────── -step "Stash any uncommitted work on current branch" - -CURRENT_BR=$(git rev-parse --abbrev-ref HEAD) -note "Currently on: $CURRENT_BR" - -# git stash --include-untracked would otherwise grab the rollout script and -# tarball (they're untracked). Move them aside, stash, move them back. -# trap EXIT guarantees restore even on script failure. -ROLLOUT_STAGE=$(mktemp -d) -ROLLOUT_FILES=(rollout-next-phase1.sh rollout-next-phase2.sh next-branch-files.tar.gz) -for f in "${ROLLOUT_FILES[@]}"; do - [ -f "./$f" ] && mv "./$f" "$ROLLOUT_STAGE/" -done -restore_rollout_files() { - # (N) is zsh's nullglob qualifier — empty match expands to nothing - # instead of erroring with "no matches found". - for f in "$ROLLOUT_STAGE"/*(N); do - cp "$f" ./ 2>/dev/null || true - done - rm -rf "$ROLLOUT_STAGE" 2>/dev/null || true -} -trap restore_rollout_files EXIT - -if [ -n "$(git status --porcelain)" ]; then - STASH_MSG="pre-next-rollout-$(date +%Y%m%d-%H%M%S) (from $CURRENT_BR)" - git stash push --include-untracked --message "$STASH_MSG" >/dev/null - ok "Stashed as: $STASH_MSG" - note "Recover later with: git stash list then git stash pop " -else - ok "Working tree clean — nothing to stash" -fi - -# Bring rollout files back into the working tree NOW so subsequent retries can find them. -restore_rollout_files - -# ─────────────────────────────────────────────────────────── -# Step 2: Switch to main, pull -# ─────────────────────────────────────────────────────────── -step "Switch to main and pull" - -git fetch origin --quiet -git checkout main >/dev/null 2>&1 || die "Could not checkout main" -git pull --ff-only origin main >/dev/null -ok "main is current at $(git log -1 --format='%h %s' | head -c 80)" - -# ─────────────────────────────────────────────────────────── -# Step 3: Create or switch to rollout branch -# ─────────────────────────────────────────────────────────── -step "Create branch $BRANCH" - -# Prune any stale worktree registrations first (e.g. from a prior aborted -# attempt that left .git/worktrees/ pointing at a deleted directory). -git worktree prune 2>/dev/null || true - -if git show-ref --verify --quiet "refs/heads/$BRANCH"; then - # Detect if a worktree still claims this branch. - # NB: no `exit` in awk — that would SIGPIPE git and trip pipefail/set -e - # silently. `head -1 || true` neutralizes the SIGPIPE we inflict ourselves. - CLAIMING_WT=$( { git worktree list --porcelain 2>/dev/null || true; } | awk -v br="refs/heads/$BRANCH" ' - /^worktree / { wt=$2 } - $0 == "branch " br { print wt } - ' | head -1 || true) - if [ -n "$CLAIMING_WT" ] && [ "$CLAIMING_WT" != "$(pwd)" ]; then - die "Branch $BRANCH is held by worktree $CLAIMING_WT. Run: git worktree remove --force '$CLAIMING_WT' (or) git worktree prune then re-run." - fi - warn "Branch $BRANCH already exists locally. Switching to it." - git checkout "$BRANCH" >/dev/null - if git rev-parse --verify "origin/$BRANCH" >/dev/null 2>&1; then - warn "origin/$BRANCH already exists. Resetting to main would lose remote commits — ABORT." - die "Delete the remote branch first if you want a clean restart: gh api -X DELETE /repos/$REPO/git/refs/heads/$BRANCH" - fi - git reset --hard main >/dev/null - ok "Reset local $BRANCH to current main" -else - git checkout -b "$BRANCH" >/dev/null - ok "Created and switched to $BRANCH" -fi - -# ─────────────────────────────────────────────────────────── -# Step 4: Unpack the 5 new files -# ─────────────────────────────────────────────────────────── -step "Unpack new files from tarball" - -tar xzf "$TARBALL" -chmod +x scripts/setup-branch-protection.sh -ok "Unpacked: $(tar tzf "$TARBALL" | wc -l | tr -d ' ') files" - -# ─────────────────────────────────────────────────────────── -# Step 5: Apply the 2 edits programmatically -# ─────────────────────────────────────────────────────────── -step "Apply edit 1/2 — add 'next' to branch-naming.yml alwaysValid" - -NAMING=".github/workflows/branch-naming.yml" -if grep -q "alwaysValid = \['main', 'next', 'develop'\]" "$NAMING"; then - ok "branch-naming.yml already has 'next' in alwaysValid (idempotent skip)" -else - if ! grep -q "alwaysValid = \['main', 'develop'\]" "$NAMING"; then - die "Could not find the expected anchor in $NAMING. Maybe upstream changed it. Open the file and add 'next' manually." - fi - # BSD sed (macOS default) needs -i ''; GNU sed needs -i. Use -i.bak then remove. - sed -i.rollout-bak "s/alwaysValid = \['main', 'develop'\]/alwaysValid = ['main', 'next', 'develop']/" "$NAMING" - rm -f "$NAMING.rollout-bak" - grep -q "alwaysValid = \['main', 'next', 'develop'\]" "$NAMING" || die "sed didn't take. Aborting." - ok "branch-naming.yml updated" -fi - -step "Apply edit 2/2 — insert 'Where Do I Open My PR?' section into CONTRIBUTING.md" - -CONTRIB="CONTRIBUTING.md" -if grep -q "^## Where Do I Open My PR?" "$CONTRIB"; then - ok "CONTRIBUTING.md already has the section (idempotent skip)" -else - # Anchor must exist exactly once. - ANCHOR_COUNT=$(grep -c "^## Pull Request Guidelines\$" "$CONTRIB" || true) - [ "$ANCHOR_COUNT" -eq 1 ] || die "Expected exactly 1 '## Pull Request Guidelines' anchor in $CONTRIB, found $ANCHOR_COUNT. Insert the section manually." - - # Write section to a temp file. BSD awk (macOS default) rejects newlines in - # -v variable values, so we pass a filename instead and let awk read it. - SECTION_FILE=$(mktemp -t gsd-rollout-section.XXXXXX) - cat > "$SECTION_FILE" <<'EOF' -## Where Do I Open My PR? (Branching Model) - -GSD uses two long-lived branches: `main` (production, what's on npm `@latest`) -and `next` (integration for the upcoming release). **Almost every PR targets -`next`.** Full guide: [`docs/branching.md`](docs/branching.md). - -| Your branch | PR target | Notes | -|---|---|---| -| `feat/NNN-slug` | `next` | Default for all new features | -| `fix/NNN-slug` | `next` | Default for all bug fixes; ships in next minor or via hotfix cherry-pick | -| `chore/`, `docs/`, `refactor/`, `test/`, `perf/`, `ci/`, `revert/` | `next` | All routine work | -| `fix/critical-NNN-slug` | `main` | Production-down emergencies only; auto-back-merges to `next` | -| `release/X.Y.0` | `main` | Created by `release.yml` — don't make these by hand | -| `hotfix/X.Y.Z` | `main` | Created by `hotfix.yml` — don't make these by hand | -| Stabilization PR for an in-flight release | `release/X.Y.0` | Fix a regression found during the RC cycle | - -**Day-to-day commands:** - -```bash -git fetch origin -git checkout next -git pull --ff-only origin next -git checkout -b fix/3187-config-corruption -# ... commit, push -gh pr create --base next --repo open-gsd/gsd-core -``` - -If you target the wrong branch by accident, the `PR Target Validator` -workflow will post a comment with the one-line fix (click "Edit" by the PR -title and change the base branch — no need to recreate the PR). - -**Why this matters:** Under the old single-branch model, every PR required -rebasing onto `main` because branch protection required "up-to-date before -merging" and `main` moved on every merge. With `next` as the integration -branch and that flag disabled on `next`, concurrent PRs can merge in any -order as long as they don't conflict on the same lines. The rebase -treadmill is gone for the 95% case. - ---- -EOF - # Trailing blank line so awk emits a blank between our closing rule and - # the next H2 (CommonMark requires blank before any header). - echo "" >> "$SECTION_FILE" - - awk -v sectionfile="$SECTION_FILE" ' - /^## Pull Request Guidelines$/ && !inserted { - while ((getline line < sectionfile) > 0) print line - close(sectionfile) - inserted = 1 - } - { print } - ' "$CONTRIB" > "$CONTRIB.tmp" - mv "$CONTRIB.tmp" "$CONTRIB" - rm -f "$SECTION_FILE" - grep -q "^## Where Do I Open My PR?" "$CONTRIB" || die "awk insertion failed." - ok "CONTRIBUTING.md section inserted" -fi - -# ─────────────────────────────────────────────────────────── -# Step 6: Sanity-check YAML + bash -# ─────────────────────────────────────────────────────────── -step "Validate YAML + bash" - -for f in .github/workflows/auto-backmerge.yml \ - .github/workflows/pr-target-validator.yml \ - .github/workflows/branch-naming.yml; do - python3 -c "import yaml,sys; yaml.safe_load(open('$f'))" \ - || die "YAML invalid: $f" -done -ok "All 3 workflow YAMLs parse" - -bash -n scripts/setup-branch-protection.sh || die "Bash syntax error: setup-branch-protection.sh" -ok "setup-branch-protection.sh syntax OK" - -# ─────────────────────────────────────────────────────────── -# Step 7: Commit -# ─────────────────────────────────────────────────────────── -step "Commit" - -git add docs/branching.md \ - docs/adr/XXXX-introduce-next-integration-branch.md \ - .github/workflows/auto-backmerge.yml \ - .github/workflows/pr-target-validator.yml \ - .github/workflows/branch-naming.yml \ - CONTRIBUTING.md \ - scripts/setup-branch-protection.sh - -if git diff --cached --quiet; then - warn "Nothing to commit (already committed from a previous run?)" -else - git commit -m "chore: introduce \`next\` integration branch (Phase 1 — additive) - -Adds: - - docs/branching.md — beginner contributor guide - - docs/adr/XXXX-...md — ADR (will be renamed with issue#) - - .github/workflows/auto-backmerge.yml — disabled in Phase 1 - - .github/workflows/pr-target-validator.yml — warn-only in Phase 1 - - scripts/setup-branch-protection.sh — idempotent gh api script - -Modifies: - - .github/workflows/branch-naming.yml — recognize 'next' - - CONTRIBUTING.md — 'Where Do I Open My PR?' section - -Phase 1 is additive: nothing operational changes until Phase 2 flips -auto-backmerge.yml's if:false→true, flips pr-target-validator.yml's -WARN_ONLY→false, creates the next branch, and switches the default -branch. See the ADR for the migration plan." - ok "Committed" -fi - -# ─────────────────────────────────────────────────────────── -# Step 8: Push -# ─────────────────────────────────────────────────────────── -step "Push branch" - -if [ "$DRY_RUN" = "1" ]; then - warn "DRY_RUN=1 — skipping push" -else - git push -u origin "$BRANCH" - ok "Pushed $BRANCH" -fi - -# ─────────────────────────────────────────────────────────── -# Step 9: File the issue (or reuse if one already exists) -# ─────────────────────────────────────────────────────────── -step "File or reuse the chore issue" - -ISSUE_TITLE="chore: introduce \`next\` integration branch to end rebase treadmill" -ISSUE_BODY=$(cat <<'EOF' -## Problem - -Every contributor branch is cut from `main` and PR'd back to `main`. Combined -with branch protection's "Require branches to be up to date before merging", -this produces a rebase treadmill: every time another PR merges, every -in-flight PR demands a rebase. With ~315 unreleased changesets queued and -multiple PRs at any time, this is constant. - -## Proposed change - -Introduce `next` as a long-lived integration branch. Routine PRs target -`next`; `main` only changes on release / hotfix / emergency fix. An -auto-back-merge workflow keeps `next` aligned with `main`. - -Full design: see the ADR in this PR (`docs/adr/XXXX-introduce-next-integration-branch.md`). -Contributor-facing guide: `docs/branching.md`. - -## Scope - -This issue covers Phase 1 of the rollout (additive infrastructure — no -operational change until the workflow flags are flipped in Phase 2). - -- New: `docs/branching.md`, ADR, `auto-backmerge.yml` (disabled), - `pr-target-validator.yml` (warn-only), `setup-branch-protection.sh`. -- Modified: `branch-naming.yml` (recognize `next`), `CONTRIBUTING.md` - ("Where Do I Open My PR?" section). -- Not yet touched: `release.yml`, `hotfix.yml`, `auto-branch.yml` — these - are Phase 3, with patches inlined in the ADR. - -## Acceptance criteria - -- All 7 files land via this PR. -- CI is green. -- `docs/branching.md` renders correctly on GitHub. -- ADR filename is renamed from `XXXX-` to `-`. - -## Phase 2 follow-up (separate PR) - -After Phase 1 merges, a small follow-up PR will: -- Create the `next` branch from `main` HEAD. -- Apply branch protection via the new script. -- Switch the default branch to `next`. -- Flip `if: false` → `if: true` in `auto-backmerge.yml`. -- Flip `WARN_ONLY: 'true'` → `'false'` in `pr-target-validator.yml`. -EOF -) - -# Look for an existing open issue with this exact title to support idempotent re-runs. -EXISTING_ISSUE=$(gh issue list --repo "$REPO" --search "in:title \"$ISSUE_TITLE\"" --state open --json number --jq '.[0].number' 2>/dev/null || echo "") - -if [ -n "$EXISTING_ISSUE" ]; then - ISSUE_NUM="$EXISTING_ISSUE" - ok "Reusing existing open issue #$ISSUE_NUM" -elif [ "$DRY_RUN" = "1" ]; then - ISSUE_NUM="DRYRUN" - warn "DRY_RUN=1 — skipping issue creation; ISSUE_NUM=$ISSUE_NUM" -else - ISSUE_URL=$(echo "$ISSUE_BODY" | gh issue create \ - --repo "$REPO" \ - --title "$ISSUE_TITLE" \ - --label "type: chore" \ - --body-file -) - ISSUE_NUM=$(echo "$ISSUE_URL" | sed 's|.*/||') - ok "Filed issue #$ISSUE_NUM — $ISSUE_URL" -fi - -# ─────────────────────────────────────────────────────────── -# Step 10: Rename ADR with issue number, replace XXXX in body -# ─────────────────────────────────────────────────────────── -step "Rename ADR and substitute issue number" - -OLD_ADR="docs/adr/XXXX-introduce-next-integration-branch.md" -NEW_ADR="docs/adr/${ISSUE_NUM}-introduce-next-integration-branch.md" - -if [ -f "$OLD_ADR" ]; then - git mv "$OLD_ADR" "$NEW_ADR" - ok "Renamed: $OLD_ADR → $NEW_ADR" -elif [ -f "$NEW_ADR" ]; then - ok "ADR already renamed (idempotent skip)" -else - die "Neither $OLD_ADR nor $NEW_ADR exists. Something is off." -fi - -# Replace XXXX inside ADR with real issue number (only in the placeholder context). -if [ "$ISSUE_NUM" != "DRYRUN" ]; then - sed -i.rollout-bak "s/XXXX-introduce-next-integration-branch/${ISSUE_NUM}-introduce-next-integration-branch/g" "$NEW_ADR" - sed -i.rollout-bak "s/placeholder \`XXXX\` prefix/placeholder (now resolved to \`${ISSUE_NUM}\`)/g" "$NEW_ADR" - rm -f "$NEW_ADR.rollout-bak" - ok "Substituted XXXX → ${ISSUE_NUM} in ADR body" -fi - -# Also patch the references in the new workflow files which mention XXXX-introduce-next-integration-branch. -for f in .github/workflows/auto-backmerge.yml .github/workflows/pr-target-validator.yml docs/branching.md CONTRIBUTING.md; do - if [ -f "$f" ] && grep -q "XXXX-introduce-next-integration-branch" "$f"; then - sed -i.rollout-bak "s/XXXX-introduce-next-integration-branch/${ISSUE_NUM}-introduce-next-integration-branch/g" "$f" - rm -f "$f.rollout-bak" - ok "Updated cross-reference in $f" - fi -done - -# ─────────────────────────────────────────────────────────── -# Step 11: Amend commit + force-push -# ─────────────────────────────────────────────────────────── -step "Amend commit and force-push" - -git add -A -if git diff --cached --quiet; then - ok "No changes to amend (idempotent skip)" -else - git commit --amend --no-edit - ok "Amended commit" -fi - -if [ "$DRY_RUN" = "1" ]; then - warn "DRY_RUN=1 — skipping force-push" -else - git push --force-with-lease origin "$BRANCH" - ok "Force-pushed (with lease)" -fi - -# ─────────────────────────────────────────────────────────── -# Step 12: Open the PR (or reuse if one already exists) -# ─────────────────────────────────────────────────────────── -step "Open PR against main" - -PR_TITLE="chore: introduce \`next\` integration branch (Phase 1 — additive)" -PR_BODY=$(cat </dev/null || echo "") - -if [ -n "$EXISTING_PR" ]; then - ok "Reusing existing PR #$EXISTING_PR" - if [ "$DRY_RUN" != "1" ]; then - echo "$PR_BODY" | gh pr edit "$EXISTING_PR" --repo "$REPO" --title "$PR_TITLE" --body-file - - ok "Updated PR #$EXISTING_PR title and body" - fi -elif [ "$DRY_RUN" = "1" ]; then - warn "DRY_RUN=1 — skipping PR creation" -else - PR_URL=$(echo "$PR_BODY" | gh pr create \ - --repo "$REPO" \ - --base main \ - --head "$BRANCH" \ - --title "$PR_TITLE" \ - --body-file -) - ok "Opened PR: $PR_URL" -fi - -# ─────────────────────────────────────────────────────────── -# Done. -# ─────────────────────────────────────────────────────────── -echo -echo "${C_BOLD}${C_GRN}━━━ Phase 1 complete ━━━${C_RST}" -echo -echo "Issue: #${ISSUE_NUM}" -echo "Branch: $BRANCH" -echo "PR: $(gh pr list --repo "$REPO" --head "$BRANCH" --state open --json url --jq '.[0].url' 2>/dev/null || echo "(check gh pr list)")" -echo -echo "Next steps:" -echo " 1. Review CI on the PR. The Changeset Required check may ask for a" -echo " changeset fragment — drop one if needed:" -echo " npm run changeset -- --type Changed --pr \\" -echo " --body \"Introduce \\\`next\\\` integration branch (Phase 1 — additive infrastructure).\"" -echo " 2. Get an approval, merge." -echo " 3. Run Phase 2: bash /path/to/rollout-next-phase2.sh" -echo -echo "Recovering your stashed work on $CURRENT_BR (if you had any):" -echo " git checkout $CURRENT_BR" -echo " git stash list # find the 'pre-next-rollout-...' entry" -echo " git stash pop stash@{N}" diff --git a/rollout-next-phase2.sh b/rollout-next-phase2.sh deleted file mode 100755 index fb45bc52c..000000000 --- a/rollout-next-phase2.sh +++ /dev/null @@ -1,300 +0,0 @@ -#!/usr/bin/env zsh -# rollout-next-phase2.sh — native zsh, runs under macOS Terminal's default shell. -# DO NOT prefix with `bash`. Run as: ./rollout-next-phase2.sh -# -# Phase 2 of the `next` integration-branch rollout. -# RUN THIS ONLY AFTER THE PHASE-1 PR HAS BEEN MERGED TO main. -# -# What it does: -# 1. Pulls main (which now contains Phase 1). -# 2. Creates the `next` branch from main HEAD, pushes it. -# 3. Applies branch protection rules via the script committed in Phase 1. -# 4. Switches the repo's default branch to `next` via gh api. -# 5. Flips `if: false` → `if: true` in auto-backmerge.yml. -# 6. Flips `WARN_ONLY: 'true'` → `'false'` in pr-target-validator.yml. -# 7. Commits the flips on a small follow-up branch, opens a PR to next. -# -# Idempotent: re-running converges. Each step checks if it's already done. -# -# Usage: -# cd /Volumes/Mini\ Me/Users/trekkie/projects/gsd-core -# bash /path/to/rollout-next-phase2.sh -# -# Env overrides: -# REPO=open-gsd/gsd-core (default) -# SKIP_PROTECTION=1 (skip running setup-branch-protection.sh) -# SKIP_DEFAULT_FLIP=1 (skip switching default branch) -# DRY_RUN=1 (skip all pushes, PRs, and api writes) - -set -euo pipefail - -REPO="${REPO:-open-gsd/gsd-core}" -FLIP_BRANCH="chore/flip-next-rollout-flags" -DRY_RUN="${DRY_RUN:-0}" - -if [ -t 1 ]; then - C_BOLD=$'\033[1m'; C_GRN=$'\033[32m'; C_YEL=$'\033[33m'; C_RED=$'\033[31m'; C_DIM=$'\033[2m'; C_RST=$'\033[0m' -else - C_BOLD=''; C_GRN=''; C_YEL=''; C_RED=''; C_DIM=''; C_RST='' -fi - -step() { echo; echo "${C_BOLD}▸ $*${C_RST}"; } -ok() { echo "${C_GRN} ✓${C_RST} $*"; } -warn() { echo "${C_YEL} ⚠${C_RST} $*"; } -die() { echo "${C_RED} ✗ $*${C_RST}" >&2; exit 1; } -note() { echo "${C_DIM} $*${C_RST}"; } - -# ─────────────────────────────────────────────────────────── -# Step 0: Sanity -# ─────────────────────────────────────────────────────────── -step "Sanity checks" - -[ -d .git ] || die "Not in a git repo. cd to your gsd-core checkout." -command -v gh >/dev/null || die "gh not found. https://cli.github.com/" -gh auth status >/dev/null 2>&1 || die "gh not authenticated." - -REMOTE_URL=$(git remote get-url origin 2>/dev/null || true) -case "$REMOTE_URL" in - *"$REPO"*) ok "Remote: $REMOTE_URL" ;; - *) die "Remote 'origin' is $REMOTE_URL — expected to contain $REPO." ;; -esac - -# Protect rollout files from the stash sweep. -ROLLOUT_STAGE=$(mktemp -d) -ROLLOUT_FILES=(rollout-next-phase1.sh rollout-next-phase2.sh next-branch-files.tar.gz) -for f in "${ROLLOUT_FILES[@]}"; do - [ -f "./$f" ] && mv "./$f" "$ROLLOUT_STAGE/" -done -restore_rollout_files() { - for f in "$ROLLOUT_STAGE"/*; do - [ -e "$f" ] || continue - cp "$f" ./ 2>/dev/null || true - done - rm -rf "$ROLLOUT_STAGE" 2>/dev/null || true -} -trap restore_rollout_files EXIT - -# Stash any in-progress work before switching branches. -CURRENT_BR=$(git rev-parse --abbrev-ref HEAD) -note "Currently on: $CURRENT_BR" -if [ -n "$(git status --porcelain)" ]; then - STASH_MSG="pre-next-phase2-$(date +%Y%m%d-%H%M%S) (from $CURRENT_BR)" - git stash push --include-untracked --message "$STASH_MSG" >/dev/null - ok "Stashed: $STASH_MSG" - STASHED=1 -else - STASHED=0 -fi - -restore_rollout_files - -# ─────────────────────────────────────────────────────────── -# Step 1: Refresh main, ensure Phase 1 is present -# ─────────────────────────────────────────────────────────── -step "Switch to main and pull" - -git fetch origin --quiet -git checkout main >/dev/null -git pull --ff-only origin main >/dev/null -ok "main is current at $(git log -1 --format='%h %s' | head -c 80)" - -# Phase-1 sentinel files must be on main now. -PHASE1_FILES=( - "docs/branching.md" - ".github/workflows/auto-backmerge.yml" - ".github/workflows/pr-target-validator.yml" - "scripts/setup-branch-protection.sh" -) -for f in "${PHASE1_FILES[@]}"; do - [ -f "$f" ] || die "Phase 1 file missing on main: $f. Has the Phase 1 PR merged?" -done -ok "Phase 1 files present on main" - -# ─────────────────────────────────────────────────────────── -# Step 2: Create next branch (idempotent) -# ─────────────────────────────────────────────────────────── -step "Create or verify the next branch" - -if git ls-remote --exit-code origin next >/dev/null 2>&1; then - ok "origin/next already exists — leaving as is" -else - if [ "$DRY_RUN" = "1" ]; then - warn "DRY_RUN=1 — would create and push next from main HEAD" - else - # Create next from current main HEAD locally and push. - git checkout -b next main 2>/dev/null || git checkout next - git push -u origin next - ok "Created and pushed origin/next at $(git log -1 --format='%h')" - # Switch back to main so subsequent steps don't accidentally edit next. - git checkout main >/dev/null - fi -fi - -# ─────────────────────────────────────────────────────────── -# Step 3: Apply branch protection -# ─────────────────────────────────────────────────────────── -step "Apply branch protection to main and next" - -if [ "${SKIP_PROTECTION:-0}" = "1" ]; then - warn "SKIP_PROTECTION=1 — skipping. You will need to run this manually:" - note " bash scripts/setup-branch-protection.sh" -elif [ "$DRY_RUN" = "1" ]; then - warn "DRY_RUN=1 — would run scripts/setup-branch-protection.sh" - REPO="$REPO" DRY_RUN=1 bash scripts/setup-branch-protection.sh | head -40 || true -else - REPO="$REPO" bash scripts/setup-branch-protection.sh - ok "Branch protection applied" -fi - -# ─────────────────────────────────────────────────────────── -# Step 4: Flip default branch to next -# ─────────────────────────────────────────────────────────── -step "Switch default branch to next" - -CURRENT_DEFAULT=$(gh api "/repos/$REPO" --jq '.default_branch') -note "Current default: $CURRENT_DEFAULT" - -if [ "$CURRENT_DEFAULT" = "next" ]; then - ok "Default is already next — skip" -elif [ "${SKIP_DEFAULT_FLIP:-0}" = "1" ]; then - warn "SKIP_DEFAULT_FLIP=1 — leaving default as $CURRENT_DEFAULT" - note "To flip later: Settings → Branches → Default branch → next" -elif [ "$DRY_RUN" = "1" ]; then - warn "DRY_RUN=1 — would PATCH /repos/$REPO default_branch=next" -else - gh api -X PATCH "/repos/$REPO" -f default_branch=next >/dev/null - ok "Default branch switched to next" -fi - -# ─────────────────────────────────────────────────────────── -# Step 5 + 6: Flip the two phase-gate flags on a follow-up branch -# ─────────────────────────────────────────────────────────── -step "Flip phase-gate flags (auto-backmerge.yml + pr-target-validator.yml)" - -# Both edits need to land on a feature branch off `next`, then PR'd back. -# (We can't push directly to main anymore — branch protection forbids it.) - -# Create or switch to the flip branch off next. -git fetch origin next --quiet 2>/dev/null || true -if git show-ref --verify --quiet "refs/heads/$FLIP_BRANCH"; then - git checkout "$FLIP_BRANCH" >/dev/null - if git rev-parse --verify "origin/$FLIP_BRANCH" >/dev/null 2>&1; then - warn "origin/$FLIP_BRANCH already exists — pulling to be sure" - git pull --ff-only origin "$FLIP_BRANCH" >/dev/null || true - else - git reset --hard origin/next >/dev/null - fi -else - git checkout -b "$FLIP_BRANCH" origin/next >/dev/null -fi - -AUTO_BM=".github/workflows/auto-backmerge.yml" -VALID=".github/workflows/pr-target-validator.yml" - -# Flip auto-backmerge: `if: false` (the phase-1 gate line) → `if: true` -# The pattern is anchored by the exact comment + indentation so we don't -# accidentally hit a different `if:` line. -if grep -q " if: false" "$AUTO_BM"; then - # macOS-compatible in-place edit - sed -i.rollout-bak "s/^ if: false$/ if: true/" "$AUTO_BM" - rm -f "$AUTO_BM.rollout-bak" - grep -q " if: true" "$AUTO_BM" || die "auto-backmerge.yml flip didn't take" - ok "auto-backmerge.yml: if: false → if: true" -elif grep -q " if: true" "$AUTO_BM"; then - ok "auto-backmerge.yml already flipped (idempotent skip)" -else - die "Could not find phase-gate 'if:' line in $AUTO_BM" -fi - -# Flip validator: WARN_ONLY: 'true' → 'false' -if grep -q "WARN_ONLY: 'true'" "$VALID"; then - sed -i.rollout-bak "s/WARN_ONLY: 'true'/WARN_ONLY: 'false'/" "$VALID" - rm -f "$VALID.rollout-bak" - grep -q "WARN_ONLY: 'false'" "$VALID" || die "validator flip didn't take" - ok "pr-target-validator.yml: WARN_ONLY 'true' → 'false'" -elif grep -q "WARN_ONLY: 'false'" "$VALID"; then - ok "pr-target-validator.yml already flipped (idempotent skip)" -else - die "Could not find WARN_ONLY in $VALID" -fi - -# YAML validation -for f in "$AUTO_BM" "$VALID"; do - python3 -c "import yaml; yaml.safe_load(open('$f'))" || die "YAML invalid after flip: $f" -done -ok "YAML still valid" - -# Commit -git add "$AUTO_BM" "$VALID" -if git diff --cached --quiet; then - warn "Nothing to commit (already committed)" -else - git commit -m "chore: enable next-branch automation (Phase 2 flips) - -- auto-backmerge.yml: enable the job (was if: false in Phase 1) -- pr-target-validator.yml: enforce instead of warn-only - -These flips are the operational gate for the next-branch model. The -next branch and branch protection were created/applied before this PR." - ok "Committed flips" -fi - -# ─────────────────────────────────────────────────────────── -# Step 7: Push + open follow-up PR against next -# ─────────────────────────────────────────────────────────── -step "Push and open follow-up PR (base = next)" - -if [ "$DRY_RUN" = "1" ]; then - warn "DRY_RUN=1 — skipping push and PR" -else - git push -u origin "$FLIP_BRANCH" - ok "Pushed $FLIP_BRANCH" - - EXISTING_PR=$(gh pr list --repo "$REPO" --head "$FLIP_BRANCH" --state open --json number --jq '.[0].number' 2>/dev/null || echo "") - PR_TITLE="chore: enable next-branch automation (Phase 2 flips)" - PR_BODY="Phase 2 follow-up to the \`next\` integration-branch rollout. - -This PR flips the two phase-gate flags that shipped inert in Phase 1: - -- \`auto-backmerge.yml\`: \`if: false\` → \`if: true\` (the back-merge job now runs on every push to main) -- \`pr-target-validator.yml\`: \`WARN_ONLY: 'true'\` → \`'false'\` (the validator now fails the check instead of just commenting) - -The \`next\` branch and branch-protection rules were created/applied out-of-band by \`scripts/rollout-next-phase2.sh\` before this PR. - -After this merges, the model is fully live. New PRs should target \`next\` (it's the default now); the validator will catch mistakes and tell contributors how to retarget." - - if [ -n "$EXISTING_PR" ]; then - echo "$PR_BODY" | gh pr edit "$EXISTING_PR" --repo "$REPO" --title "$PR_TITLE" --body-file - - ok "Updated existing PR #$EXISTING_PR" - else - PR_URL=$(echo "$PR_BODY" | gh pr create \ - --repo "$REPO" \ - --base next \ - --head "$FLIP_BRANCH" \ - --title "$PR_TITLE" \ - --body-file -) - ok "Opened PR: $PR_URL" - fi -fi - -# ─────────────────────────────────────────────────────────── -# Done -# ─────────────────────────────────────────────────────────── -echo -echo "${C_BOLD}${C_GRN}━━━ Phase 2 complete ━━━${C_RST}" -echo -echo "What's live now:" -echo " • next branch exists, branch protection applied to main + next" -echo " • Default branch: next (new PRs default to it)" -echo " • auto-backmerge.yml: enabled — will open main→next PR after each push to main" -echo " • pr-target-validator.yml: enforcing — fails the check on wrong target" -echo -echo "Follow-up PR to merge: $(gh pr list --repo "$REPO" --head "$FLIP_BRANCH" --state open --json url --jq '.[0].url' 2>/dev/null || echo "(check gh pr list)")" -echo -echo "Phase 3 (when ready, weeks-to-months out): apply the release.yml /" -echo "hotfix.yml / auto-branch.yml patches inlined in the ADR." -if [ "$STASHED" = "1" ]; then - echo - echo "Your earlier work is stashed. Recover it with:" - echo " git checkout $CURRENT_BR && git stash list && git stash pop stash@{0}" -fi diff --git a/scripts/ci-prepare-test-scope.cjs b/scripts/ci-prepare-test-scope.cjs index 66309db1a..90d5aef50 100644 --- a/scripts/ci-prepare-test-scope.cjs +++ b/scripts/ci-prepare-test-scope.cjs @@ -16,13 +16,37 @@ const path = require('path'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); -const scope = process.env.TEST_SCOPE || ''; -const targeted = process.env.TARGETED_TESTS || ''; -const windows = process.env.WINDOWS_TESTS || ''; +// Suite sentinels understood by run-tests.cjs (scripts/run-tests.cjs SUITES). +// A sentinel resolves to its live file set at run time, so — unlike an explicit +// filename — it can never reference a since-deleted test file. +const SUITE_SENTINELS = ['all', 'unit', 'integration', 'install', 'security', 'slow']; -const FALLBACK = 'tests/command-contract.test.cjs tests/commands.test.cjs tests/core.test.cjs tests/package-manifest.test.cjs'; +// Fast smoke set used when scope detection yields no targeted tests. Each entry +// MUST resolve via run-tests.cjs: an existing repo-relative test file or a +// SUITE_SENTINELS token. A stale filename here (a test deleted by a refactor) +// is exactly what broke CI in #1329 — `tests/core.test.cjs`, deleted in #1291, +// was still listed and crashed every scoped lane that hit this fallback. The +// parity guard in tests/bug-641-files-from-suite-token.test.cjs fails the +// moment an entry stops resolving; resolveSelection() filters at write time so +// a stale entry degrades instead of crashing the lane. +const FALLBACK = [ + 'tests/command-contract.test.cjs', + 'tests/commands.test.cjs', + 'tests/package-manifest.test.cjs', +]; -function main() { +// Last-resort selector when every FALLBACK entry has been deleted: the whole +// unit suite, resolved live by run-tests.cjs (the #408/#641 sentinel path). +const FALLBACK_SENTINEL = 'unit'; + +// An entry is runnable if it is a suite sentinel or an existing file under root. +function isResolvable(entry, root) { + return SUITE_SENTINELS.includes(entry) || fs.existsSync(path.join(root, entry)); +} + +// Resolve the scoped test selection for the lane. Pure (no I/O beyond the +// existence probe under `root`) so it can be unit-tested directly. +function resolveSelection({ scope, targeted, windows, root }) { let selected; if (scope === 'windows') { selected = windows; @@ -32,20 +56,38 @@ function main() { throw new ExitError(1, `::error::Unknown test scope: ${scope}`); } - // Trim and fall back to default set if empty. - if (!selected.trim()) { - selected = FALLBACK; + // Detected list passes through verbatim: affected-tests-lib.cjs already filters + // deleted files, and the list may legitimately carry a suite sentinel. + const detected = (selected || '').split(/\s+/).filter(Boolean); + if (detected.length > 0) { + return detected; } - // Split on whitespace, filter blanks, join with newlines. - const lines = selected.split(/\s+/).filter(Boolean); - const content = lines.join('\n') + '\n'; + // Empty detection → smoke fallback, existence-filtered so a stale entry can + // never crash the scoped lane (#1329). If nothing survives, use the unit + // sentinel, which run-tests.cjs always resolves. + const survivors = FALLBACK.filter((f) => isResolvable(f, root)); + return survivors.length > 0 ? survivors : [FALLBACK_SENTINEL]; +} - const outPath = path.join(process.cwd(), '.ci-selected-tests.txt'); - fs.writeFileSync(outPath, content, 'utf-8'); +function main() { + const root = process.cwd(); + const lines = resolveSelection({ + scope: process.env.TEST_SCOPE || '', + targeted: process.env.TARGETED_TESTS || '', + windows: process.env.WINDOWS_TESTS || '', + root, + }); + + const content = lines.join('\n') + '\n'; + fs.writeFileSync(path.join(root, '.ci-selected-tests.txt'), content, 'utf-8'); process.stdout.write('Scoped tests:\n'); process.stdout.write(content); } -runMain(main); +if (require.main === module) { + runMain(main); +} + +module.exports = { FALLBACK, FALLBACK_SENTINEL, SUITE_SENTINELS, resolveSelection, main }; diff --git a/scripts/diff-touches-shipped-paths.cjs b/scripts/diff-touches-shipped-paths.cjs index da0943d60..ae27dc64c 100644 --- a/scripts/diff-touches-shipped-paths.cjs +++ b/scripts/diff-touches-shipped-paths.cjs @@ -12,11 +12,10 @@ * - package.json (always included by `npm pack`, regardless of `files`) * - every entry in package.json `files`, treated as either an exact * file match or a directory prefix (matching `npm pack` semantics). - * - CI-gating test paths: `tests/` plus - * `sdk/src//.test.` and `.spec.` variants - * — these don't ship in the tarball, but they gate the hotfix-branch - * test job. A test fixture update that aligns with a cherry-picked - * production fix MUST be pickable or CI fails on the hotfix run. + * - CI-gating test paths: `tests/` — these don't ship in the + * tarball, but they gate the hotfix-branch test job. A test fixture + * update that aligns with a cherry-picked production fix MUST be + * pickable or CI fails on the hotfix run. * #3621 — root cause of the v1.42.3 hotfix red CI. * * `package-lock.json` is intentionally NOT considered shipped — `npm pack` @@ -65,12 +64,7 @@ function loadShipPrefixes(pkgPath) { // in hotfix.yml, this lets `test(####):` fixture-alignment commits be // cherry-picked alongside their production counterparts. function isCiGating(diffPath) { - if (diffPath.startsWith('tests/')) return true; - // SDK vitest specs live next to source. Production source ships via - // sdk/dist/ (already in package.json `files`); the test files are what's - // missing from that surface. - if (diffPath.startsWith('sdk/src/') && /\.(test|spec)\.(ts|cjs|mjs|js)$/.test(diffPath)) return true; - return false; + return diffPath.startsWith('tests/'); } function isShipped(diffPath, shipPrefixes) { diff --git a/src/capability-activation.cts b/src/capability-activation.cts index 1440dc69f..265f73a49 100644 --- a/src/capability-activation.cts +++ b/src/capability-activation.cts @@ -62,39 +62,71 @@ function _readRawConfigKey( } } +/** + * Resolve the raw value for a dotted config key using the four-level precedence + * walk. Returns { found, value } with the RAW value (not coerced to boolean), + * so callers can decide how to interpret the value (boolean gate vs. raw config + * value for numeric/string settings like security_asvs_level). + * + * Precedence (mirrors _resolveActivationValue): + * 1. loadConfig result (config arg) — guarded nested-lookup. + * 2. Workstream config.json at planningDir(cwd)/config.json. + * 3. Root config.json at planningRoot(cwd)/config.json (only if path differs). + * 4. registry.configSchema[dotKey].default — schema default. + * 5. Absent → { found: false, value: undefined }. + */ +function resolveConfigKey( + dotKey: string, + opts: { config: Record; cwd: string | undefined; registry: Record }, +): { found: boolean; value: unknown } { + const { config, cwd, registry } = opts; + + // Level 1: loadConfig result + const fromConfig = _getNestedConfigValue(config, dotKey); + if (fromConfig.found) return { found: true, value: fromConfig.value }; + + // Level 2 + 3: raw config.json files (only when cwd is available) + if (cwd) { + const wsConfigPath = path.join(planningDir(cwd), 'config.json'); + const rootConfigPath = path.join(planningRoot(cwd), 'config.json'); + + const fromWs = _readRawConfigKey(wsConfigPath, dotKey); + if (fromWs.found) return { found: true, value: fromWs.value }; + + if (wsConfigPath !== rootConfigPath) { + const fromRoot = _readRawConfigKey(rootConfigPath, dotKey); + if (fromRoot.found) return { found: true, value: fromRoot.value }; + } + } + + // Level 4: registry configSchema default + const schemaMap = registry['configSchema']; + if (schemaMap && typeof schemaMap === 'object' && !Array.isArray(schemaMap) + && Object.prototype.hasOwnProperty.call(schemaMap, dotKey)) { + const schemaEntry = (schemaMap as Record)[dotKey]; + if (schemaEntry && typeof schemaEntry === 'object' && schemaEntry !== null) { + const def = (schemaEntry as Record)['default']; + if (def !== undefined) return { found: true, value: def }; + } + } + + // Level 5: absent + return { found: false, value: undefined }; +} + function _resolveActivationValue( dotKey: string, config: Record, cwd: string | undefined, registry: Record, ): boolean { - const fromConfig = _getNestedConfigValue(config, dotKey); - if (fromConfig.found) return Boolean(fromConfig.value); - - if (cwd) { - const wsConfigPath = path.join(planningDir(cwd), 'config.json'); - const rootConfigPath = path.join(planningRoot(cwd), 'config.json'); - - const fromWs = _readRawConfigKey(wsConfigPath, dotKey); - if (fromWs.found) return Boolean(fromWs.value); - - if (wsConfigPath !== rootConfigPath) { - const fromRoot = _readRawConfigKey(rootConfigPath, dotKey); - if (fromRoot.found) return Boolean(fromRoot.value); - } - } - - const schemaEntry = (registry['configSchema'] as Record | undefined)?.[dotKey]; - if (schemaEntry && typeof schemaEntry === 'object' && schemaEntry !== null) { - const def = (schemaEntry as Record)['default']; - if (def !== undefined) return Boolean(def); - } - - return false; + const r = resolveConfigKey(dotKey, { config, cwd, registry }); + return r.found ? Boolean(r.value) : false; } export = { _getNestedConfigValue, _readRawConfigKey, _resolveActivationValue, + resolveConfigKey, }; diff --git a/src/capability-state.cts b/src/capability-state.cts index c981af516..81e1d8c83 100644 --- a/src/capability-state.cts +++ b/src/capability-state.cts @@ -128,8 +128,6 @@ interface ResolveCapabilityStateResult { interface ResolveCapabilityRuntimeStateResult { runtimeConfigDir: string; warnings: string[]; - registry: Record; - config: Record; capabilities: CapabilityStateEntry[]; } @@ -430,6 +428,7 @@ function _resolveManifest(commandsGsdDir: string, configDir: string): Map, ): ResolveCapabilityRuntimeStateResult { const warnings: string[] = []; @@ -520,11 +519,18 @@ function resolveCapabilityRuntimeState( } // ── Load config ─────────────────────────────────────────────────────────────── + // When the caller already holds a loadConfig snapshot (e.g. cmdLoopRenderHooks), + // accept it via configOverride so capability `active` and hook resolution + // share the SAME config object — single snapshot, no TOCTOU window. let config: Record; - try { - config = loadConfig(cwd); - } catch { - config = {}; + if (configOverride !== undefined) { + config = configOverride; + } else { + try { + config = loadConfig(cwd); + } catch { + config = {}; + } } // ── Resolve state ──────────────────────────────────────────────────────────── @@ -540,8 +546,6 @@ function resolveCapabilityRuntimeState( return { runtimeConfigDir: resolvedConfigDir, warnings, - registry, - config, capabilities: result.capabilities, }; } diff --git a/src/capability-writer.cts b/src/capability-writer.cts index 37d178b81..cf067eaee 100644 --- a/src/capability-writer.cts +++ b/src/capability-writer.cts @@ -129,7 +129,8 @@ function setCapabilityState( const resolvedConfigDir = before.runtimeConfigDir; // ── Load registry ───────────────────────────────────────────────────────── - const registry = before.registry; + // eslint-disable-next-line @typescript-eslint/no-require-imports + const registry = require('./capability-registry.cjs') as Record; const capabilitiesMap = ( registry['capabilities'] && typeof registry['capabilities'] === 'object' && !Array.isArray(registry['capabilities']) ? registry['capabilities'] diff --git a/src/loop-resolver.cts b/src/loop-resolver.cts index a566f6c38..fe07faeb4 100644 --- a/src/loop-resolver.cts +++ b/src/loop-resolver.cts @@ -20,17 +20,14 @@ * registry/config arguments so they are trivially testable without I/O. * * Dependencies (leaf modules only — no circular risk): - * - node:fs / node:path (raw config.json read for capability-key activation) * - ./config-loader.cjs (loadConfig) - * - ./planning-workspace.cjs (planningDir — to locate config.json) * - ./io.cjs (output, error) + * - ./capability-activation.cjs (resolveConfigKey, _resolveActivationValue, _getNestedConfigValue, _readRawConfigKey) * - loop-host-contract.cjs (CANONICAL_POINTS via LOOP_HOST_CONTRACT) * - capability-registry.cjs (byLoopPoint, consumed at call time) + * - capability-state.cjs (resolveCapabilityRuntimeState — for capabilities list) */ -import fs from 'node:fs'; -import path from 'node:path'; - // eslint-disable-next-line @typescript-eslint/no-require-imports import ioMod = require('./io.cjs'); const { output: coreOutput, error: coreError } = ioMod; @@ -43,9 +40,10 @@ const { loadConfig } = configLoaderModule; import capabilityStateModule = require('./capability-state.cjs'); const { resolveCapabilityRuntimeState } = capabilityStateModule; +// ─── Capability-activation engine (single owner for config-key precedence) ──── // eslint-disable-next-line @typescript-eslint/no-require-imports -import planningWorkspaceMod = require('./planning-workspace.cjs'); -const { planningDir, planningRoot } = planningWorkspaceMod; +import capabilityActivationModule = require('./capability-activation.cjs'); +const { _getNestedConfigValue, _readRawConfigKey, _resolveActivationValue, resolveConfigKey } = capabilityActivationModule; // ─── Canonical points (derived from LOOP_HOST_CONTRACT — authoritative 12) ─── @@ -95,133 +93,7 @@ function _getCanonicalPoints(_registry: Record): ReadonlyArray< return CANONICAL_POINTS; } -// ─── Prototype-pollution guard (inline literal, CodeQL barrier) ─────────────── - -/** - * Traverse a dotted config key through a nested config object. - * E.g. "workflow.ui_phase" in { workflow: { ui_phase: true } } → { found: true, value: true } - * Returns { found: false } if any segment is a forbidden key or not an own property. - */ -function _getNestedConfigValue( - config: Record, - dotKey: string, -): { found: boolean; value: unknown } { - const segments = dotKey.split('.'); - let current: unknown = config; - for (const seg of segments) { - // Inline literal prototype-pollution guard (CodeQL barrier) - if (seg === '__proto__' || seg === 'constructor' || seg === 'prototype') { - return { found: false, value: undefined }; - } - if (typeof current !== 'object' || current === null) { - return { found: false, value: undefined }; - } - const cur = current as Record; - if (!Object.prototype.hasOwnProperty.call(cur, seg)) { - return { found: false, value: undefined }; - } - current = cur[seg]; - } - return { found: true, value: current }; -} - -// ─── Single-key activation resolver (FIX 1) ─────────────────────────────────── - -/** - * Warn-once set for raw config.json parse errors. - * Avoids noisy per-call stderr from a single malformed file. - */ -const _warnedRawConfigPaths = new Set(); - -/** - * Read a raw config.json file and perform a guarded nested-lookup of a single - * dotted key. Returns { found: false } if the file is missing (ENOENT) or if - * the key is absent/forbidden. On a genuine JSON parse error: warns once to - * stderr and returns { found: false } — never throws. - */ -function _readRawConfigKey( - filePath: string, - dotKey: string, -): { found: boolean; value: unknown } { - try { - const raw = fs.readFileSync(filePath, 'utf8'); - let parsed: Record; - try { - parsed = JSON.parse(raw) as Record; - } catch { - if (!_warnedRawConfigPaths.has(filePath)) { - _warnedRawConfigPaths.add(filePath); - try { - process.stderr.write( - `gsd-tools: warning: failed to parse ${filePath} as JSON — skipping for activation resolution\n`, - ); - } catch { /* stderr might be closed */ } - } - return { found: false, value: undefined }; - } - return _getNestedConfigValue(parsed, dotKey); - } catch { - // ENOENT (missing file) is expected → skip silently. All other errors → also skip (defensive). - return { found: false, value: undefined }; - } -} - -/** - * FIX 1: Resolve the effective value for a hook's `when` key using the - * four-level precedence: - * - * 1. loadConfig result (`config` arg) — guarded nested-lookup of the dotted key. - * This is the post-cutover federated path (covers keys that loadConfig now exposes). - * 2. Raw workstream `.planning/.../config.json` — guarded single-key lookup. - * Workstream wins over root (mirrors loadConfig inheritance). - * 3. Raw root `.planning/config.json` — guarded single-key lookup. - * 4. `registry.configSchema[when]?.default` — schema default. - * A `default: true` hook is active out-of-the-box without any config. - * 5. Absent → inactive (return false). - * - * Never constructs a merged object from raw JSON keys — only reads the single - * leaf value at the guarded dotted path. Prototype-pollution sink is eliminated. - */ -function _resolveActivationValue( - dotKey: string, - config: Record, - cwd: string | undefined, - registry: Record, -): boolean { - // Level 1: loadConfig result - const fromConfig = _getNestedConfigValue(config, dotKey); - if (fromConfig.found) return Boolean(fromConfig.value); - - // Level 2 + 3: raw config.json files (only when cwd is available) - if (cwd) { - // Level 2: workstream config (planningDir respects GSD_WORKSTREAM env) - const wsConfigPath = path.join(planningDir(cwd), 'config.json'); - // Level 3: root config (planningRoot = cwd/.planning always) - const rootConfigPath = path.join(planningRoot(cwd), 'config.json'); - - // Workstream wins over root (mirroring loadConfig root→workstream precedence: - // workstream overlays root, so workstream value takes precedence). - const fromWs = _readRawConfigKey(wsConfigPath, dotKey); - if (fromWs.found) return Boolean(fromWs.value); - - // Only read root if it differs from the workstream path (avoids double-read - // when no workstream is active and both paths resolve to the same file). - if (wsConfigPath !== rootConfigPath) { - const fromRoot = _readRawConfigKey(rootConfigPath, dotKey); - if (fromRoot.found) return Boolean(fromRoot.value); - } - } - - // Level 4: registry configSchema default - const schemaEntry = (registry['configSchema'] as Record | undefined)?.[dotKey]; - if (schemaEntry && typeof schemaEntry === 'object' && schemaEntry !== null) { - const def = (schemaEntry as Record)['default']; - if (def !== undefined) return Boolean(def); - } - - // Level 5: absent → inactive - return false; -} +// ─── (Precedence engine imported from capability-activation.cjs above) ──────── // ─── Types ──────────────────────────────────────────────────────────────────── @@ -383,27 +255,8 @@ function resolveLoopHooks(input: ResolveLoopHooksInput): ResolveLoopHooksResult // Prototype-pollution guard (inline literal, CodeQL barrier) if (alias === '__proto__' || alias === 'constructor' || alias === 'prototype') continue; if (typeof dotKey !== 'string') continue; - // Level 1: loadConfig result - const fromConfig = _getNestedConfigValue(config, dotKey); - if (fromConfig.found) { resolved[alias] = fromConfig.value; continue; } - // Level 2 + 3: raw config.json files - if (cwd) { - const wsConfigPath = path.join(planningDir(cwd), 'config.json'); - const rootConfigPath = path.join(planningRoot(cwd), 'config.json'); - const fromWs = _readRawConfigKey(wsConfigPath, dotKey); - if (fromWs.found) { resolved[alias] = fromWs.value; continue; } - if (wsConfigPath !== rootConfigPath) { - const fromRoot = _readRawConfigKey(rootConfigPath, dotKey); - if (fromRoot.found) { resolved[alias] = fromRoot.value; continue; } - } - } - // Level 4: registry configSchema default - const schemaEntry = (registry['configSchema'] as Record | undefined)?.[dotKey]; - if (schemaEntry && typeof schemaEntry === 'object' && schemaEntry !== null) { - const def = (schemaEntry as Record)['default']; - if (def !== undefined) { resolved[alias] = def; continue; } - } - // Level 5: absent → undefined (omit from resolved map) + const r = resolveConfigKey(dotKey, { config, cwd, registry }); + if (r.found) resolved[alias] = r.value; } return Object.keys(resolved).length > 0 ? resolved : undefined; } @@ -622,14 +475,26 @@ function cmdLoopRenderHooks( const runtimeConfigDir = typeof options['configDir'] === 'string' ? options['configDir'] : undefined; - const state = resolveCapabilityRuntimeState(cwd, runtimeConfigDir) as { + // Load the config snapshot ONCE and share it with both the capability-state + // resolver (via configOverride) and loop-hook resolution, so federated keys + // present in loadConfig resolve identically for `active` and for hook when/ + // configValues — eliminating the previous double loadConfig() call. Note: keys + // absent from loadConfig still fall through to raw .planning/config.json reads + // (precedence levels 2-3) in each pass; that residual re-read window is + // pre-existing (unchanged by this consolidation), not introduced here. + let config: Record; + try { + config = loadConfig(cwd); + } catch { + config = {}; + } + const state = resolveCapabilityRuntimeState(cwd, runtimeConfigDir, config) as { warnings?: string[]; - registry: Record; - config: Record; capabilities: Array<{ id: string; enabled?: boolean; active: boolean }>; }; - const registry = state.registry; - const config = state.config || loadConfig(cwd); + // Registry is the static generated module — same object capability-state uses internally. + // eslint-disable-next-line @typescript-eslint/no-require-imports + const registry = require('./capability-registry.cjs') as Record; const capabilityStatesById = new Map(); for (const cap of state.capabilities || []) { capabilityStatesById.set(cap.id, cap); @@ -677,6 +542,9 @@ export = { _getNestedConfigValue, _resolveActivationValue, _readRawConfigKey, + // Re-exported for identity parity guard (FIX 2: resolveConfigValues in this module + // calls resolveConfigKey; exporting it here makes the single-owner contract testable). + resolveConfigKey, CANONICAL_POINTS_FALLBACK, CANONICAL_POINTS, }; diff --git a/src/roadmap-parser.cts b/src/roadmap-parser.cts index 3d5fcf100..7452def93 100644 --- a/src/roadmap-parser.cts +++ b/src/roadmap-parser.cts @@ -216,6 +216,33 @@ interface RoadmapPhaseResult { section: string; } +function findRoadmapPhaseInContent(content: string, phaseNum: unknown): RoadmapPhaseResult | null { + const phasePattern = new RegExp( + `#{2,4}\\s*(?:\\[[^\\]]+\\]\\s*)?Phase\\s+${phaseMarkdownRegexSource(phaseNum)}:\\s*([^\\n]+)`, + 'i' + ); + const headerMatch = content.match(phasePattern); + if (!headerMatch) return null; + + const phaseName = headerMatch[1].trim(); + const headerIndex = headerMatch.index!; + const restOfContent = content.slice(headerIndex); + const nextHeaderMatch = restOfContent.match(/\n#{2,4}\s+(?:\[[^\]]+\]\s*)?Phase\s+[\w]/i); + const sectionEnd = nextHeaderMatch ? headerIndex + nextHeaderMatch.index! : content.length; + const section = content.slice(headerIndex, sectionEnd).trim(); + + const goalMatch = section.match(/\*\*Goal(?:\*\*:|\*?\*?:\*\*)\s*([^\n]+)/i); + const goal = goalMatch ? goalMatch[1].trim() : null; + + return { + found: true, + phase_number: String(phaseNum), + phase_name: phaseName, + goal, + section, + }; +} + function getRoadmapPhaseInternal(cwd: string, phaseNum: unknown): RoadmapPhaseResult | null { if (!phaseNum) return null; const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md'); @@ -225,31 +252,10 @@ function getRoadmapPhaseInternal(cwd: string, phaseNum: unknown): RoadmapPhaseRe const roadmapRaw = platformReadSync(roadmapPath); if (roadmapRaw === null) throw new Error('missing'); const content = extractCurrentMilestone(roadmapRaw, cwd); - const phasePattern = new RegExp( - `#{2,4}\\s*(?:\\[[^\\]]+\\]\\s*)?Phase\\s+${phaseMarkdownRegexSource(phaseNum)}:\\s*([^\\n]+)`, - 'i' - ); - const headerMatch = content.match(phasePattern); - if (!headerMatch) return null; + const scopedResult = findRoadmapPhaseInContent(content, phaseNum); + if (scopedResult) return scopedResult; - const phaseName = headerMatch[1].trim(); - const headerIndex = headerMatch.index!; - const restOfContent = content.slice(headerIndex); - const nextHeaderMatch = restOfContent.match(/\n#{2,4}\s+(?:\[[^\]]+\]\s*)?Phase\s+[\w]/i); - const sectionEnd = nextHeaderMatch ? headerIndex + nextHeaderMatch.index! : content.length; - const section = content.slice(headerIndex, sectionEnd).trim(); - - const goalMatch = section.match(/\*\*Goal(?:\*\*:|\*?\*?:\*\*)\s*([^\n]+)/i); - const goal = goalMatch ? goalMatch[1].trim() : null; - - return { - found: true, - // eslint-disable-next-line @typescript-eslint/no-base-to-string - phase_number: String(phaseNum), - phase_name: phaseName, - goal, - section, - }; + return findRoadmapPhaseInContent(stripShippedMilestones(roadmapRaw), phaseNum); } catch { return null; } diff --git a/src/verify.cts b/src/verify.cts index ad3583865..0ba8ad168 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -253,6 +253,310 @@ function scanNegativeGrepCommentEcho(content: string): { errors: string[]; warni return { errors, warnings }; } +/** + * Issue #968 — file-wide negative-grep sibling conflict detector. + * A file-wide negative grep gate (! grep -Eq 'PAT' FILE or grep -c 'PAT' FILE == 0) + * bans a construct across the WHOLE file. When a sibling task in the same plan + * legitimately requires the same construct in the same file, the two gates are + * mutually unsatisfiable. This is a WARN-only check (never changes valid:false). + */ +function scanFileWideNegativeGateConflict(content: string): { warnings: string[]; valid: true } { + const warnings: string[] = []; + + // Normalize newlines; join backslash line-continuations (same as #429). + const text = (content || '') + .replace(/\r\n/g, '\n') + .replace(/\r/g, '\n') + .replace(/\\\n/g, ' '); + + // Allowlisted patterns: + const allow = new Set(); + const allowRe = //g; + let am: RegExpExecArray | null; + while ((am = allowRe.exec(text)) !== null) allow.add(am[1]); + + // Helper predicates (reused from #429 style). + // Zero-equality comparison: spaced == 0 or -eq 0. + const zeroCmp = (s: string): boolean => + /\s==?\s*0\b/.test(s) || /-eq\s+0\b/.test(s) || /\bequals\s+0\b/.test(s); + + // grep options include -c / --count + const optsHaveCount = (opts: string): boolean => + /(?:^|\s)-[A-Za-z]*c[A-Za-z]*(?=\s|$)/.test(opts) || /--count\b/.test(opts); + + // grep options include -v / --invert-match (inverted count is NOT a negative gate) + const optsHaveInvert = (opts: string): boolean => + /(?:^|\s)-[A-Za-z]*v[A-Za-z]*(?=\s|$)/.test(opts) || /--invert-match\b/.test(opts); + + // A bareword that is a plausible grep pattern (not a stray flag/number). + const plausibleBare = (s: string): boolean => /[A-Za-z0-9_]/.test(s) && !/^[-=!<>0-9]+$/.test(s); + + // Regex to extract grep arguments: opts run then PAT (quoted or bare). + const grepArgRe = + /grep((?:\s+-{1,2}[A-Za-z][A-Za-z-]*)+)\s+(?:'([^']*)'|"([^"]*)"|([^\s'"|>&;$()\[\]]+))/g; + + // FIX 1 (ReDoS): Linear-time "does reqText satisfy the grep pattern" — no RegExp execution. + // Never calls new RegExp, so no catastrophic backtracking is possible. + // + // Handles literal patterns and `.`/`.*/`.+`/`\s`-style wildcard gaps and `^`/`$` anchors. + // Patterns using character classes (`[…]`), alternation (`a|b`), or other regex constructs + // fall back to a conservative literal-substring check, so the detector may NOT warn on those + // (false-negative is the safe direction for a warn-only advisory). + const patternRequiredIn = (pat: string, reqText: string): boolean => { + const hay = (reqText || '').slice(0, 8000); // bound the haystack + if (!pat) return false; + // Strip ERE anchors — position constraints don't change whether the construct is required. + pat = pat.replace(/^\^/, '').replace(/\$$/, ''); + if (!pat) return false; + // Pure literal (no regex metacharacters): direct substring. + if (!/[.*+?^${}()|[\]\\]/.test(pat)) return hay.includes(pat); + const SENT = ' '; + // Replace simple wildcard gaps (\s* \w+ .* .+ .? bare .) with a sentinel. + let work = pat + .replace(/\\[sSwWdD][*+?]?/g, SENT) + .replace(/\.[*+?]/g, SENT) + .replace(/\./g, SENT); + work = work.replace(/\\(.)/g, '$1'); // de-escape \( \. etc → literal char + const joined = work.split(SENT).join(''); + // Unhandled regex constructs remain → safe literal-substring fallback on the raw pattern. + if (/[*+?^${}()|[\]]/.test(joined)) return hay.includes(pat); + const frags = work.split(SENT).filter(Boolean); + if (!frags.length) return false; // all-wildcard pattern → no meaningful requirement + let pos = 0; + for (const f of frags) { + const idx = hay.indexOf(f, pos); + if (idx === -1) return false; + pos = idx + f.length; + } + return true; + }; + + // FIX 2 (file basename over-match): exact normalized match; basename fallback ONLY for + // unqualified gate files (no path separator). + const normPath = (p: string): string => p.replace(/^\.\//, '').trim(); + + // File-wide discriminator: a token AFTER PAT that looks like a path. + // Paths have /, a file extension, or match a known task entry. + // Globs (containing *) are excluded (unresolvable — no warn). + const looksLikePath = (token: string): boolean => + !token.includes('*') && + (token.includes('/') || /\.[a-zA-Z]{1,6}$/.test(token)); + + // FIX 5 (hasLeadingNot): collapse to one command-boundary-anchored regex. + // Negation at a command boundary: start of segment, or after ; & | ( newline / then / do. + + // FIX 4 (isRegionScoped tightened): return true ONLY when grep is downstream of a + // sed line-range or awk range producer. Other pipe sources (cat, tac, etc.) are file-wide. + const isRegionScoped = (seg: string): boolean => { + if (!seg.includes('|')) return false; + const before = seg.slice(0, seg.lastIndexOf('|')); + // sed -n line/range extraction, e.g. sed -n '12,40p' FILE or sed -n '/a/,/b/p' FILE + if (/\bsed\s+-n\b/.test(before)) return true; + // awk range pattern, e.g. awk '/start/,/end/' FILE + if (/\bawk\b[^|]*\/[^/]*\/\s*,\s*\/[^/]*\//.test(before)) return true; + return false; + }; + + // Parse all blocks. + interface TaskInfo { + name: string; + files: string[]; // entries from + gateText: string; // ++ text + reqText: string; // + text (requirement side) + } + const taskRe = /]*>([\s\S]*?)<\/task>/g; + const tasks: TaskInfo[] = []; + let tm: RegExpExecArray | null; + while ((tm = taskRe.exec(text)) !== null) { + const tc = tm[1]; + // Extract task name. + const namem = tc.match(/([\s\S]*?)<\/name>/); + const name = namem ? namem[1].trim() : 'unnamed'; + // Extract entries. + const filesm = tc.match(/([\s\S]*?)<\/files>/); + const filesText = filesm ? filesm[1] : ''; + const files = filesText.split(/[,\s]+/).map(s => s.trim()).filter(Boolean); + // Gate text: //. + const gateFragments: string[] = []; + for (const tag of ['verify', 'automated', 'acceptance_criteria']) { + const re = new RegExp(`<${tag}>([\\s\\S]*?)<\\/${tag}>`, 'g'); + let mm: RegExpExecArray | null; + while ((mm = re.exec(tc)) !== null) gateFragments.push(mm[1]); + } + // Requirement text: /. + const reqFragments: string[] = []; + for (const tag of ['action', 'acceptance_criteria']) { + const re = new RegExp(`<${tag}>([\\s\\S]*?)<\\/${tag}>`, 'g'); + let mm: RegExpExecArray | null; + while ((mm = re.exec(tc)) !== null) reqFragments.push(mm[1]); + } + // Strip XML tags from gate text so segments containing embedded + // XML closing tags (e.g. cmd nested inside ) + // don't bleed into the file-path token extraction. + const rawGateText = gateFragments.join('\n'); + const gateText = rawGateText.replace(/<[^>]+>/g, ' '); + tasks.push({ + name, + files, + gateText, + reqText: reqFragments.join('\n'), + }); + } + + if (tasks.length < 2) return { warnings, valid: true }; + + // FIX 3 (extensionless known files): build a normalized set of ALL tasks' entries + // so that extensionless filenames like Dockerfile are also recognized as valid file tokens. + const knownFiles = new Set(); + for (const t of tasks) { + for (const f of t.files) knownFiles.add(normPath(f)); + } + + // Extended looksLikePath: accepts known entries even without an extension. + const isFileLike = (token: string): boolean => { + if (token.includes('*')) return false; // exclude globs + if (looksLikePath(token)) return true; + return knownFiles.has(normPath(token)); + }; + + // Dedup key: (taskAIdx, taskBIdx, pat, file) + const seen = new Set(); + + // For each task A, scan gate text for file-wide negative grep bans. + for (let ai = 0; ai < tasks.length; ai++) { + const taskA = tasks[ai]; + + // Split gate text into shell segments (split on && / || within lines). + const segments = taskA.gateText.split('\n').flatMap(line => + line.split(/\s*(?:&&|\|\|)\s*/), + ); + + for (const seg of segments) { + if (!/grep/.test(seg)) continue; + + // FIX 5: Negation at a command boundary: start of segment, or after ; & | ( newline / then / do. + // Also handles ! negating an entire pipeline (e.g. ! cat FILE | grep ...). + const hasLeadingNot = + // Direct ! grep: negation immediately before grep keyword + /(?:^|[\n;&|(]|\bthen\b|\bdo\b)\s*!\s*grep/.test(seg) || + // Pipeline negation: ! at command boundary, grep appears in pipeline after | + (/(?:^|[\n;&|(]|\bthen\b|\bdo\b)\s*!\s*\w/.test(seg) && /\|\s*grep\b/.test(seg)); + + const hasCountZero = zeroCmp(seg); + + // Extract grep invocation and check for count. + grepArgRe.lastIndex = 0; + let pat: string | null = null; + let file: string | null = null; + let isBan = false; + + // FIX 4 helper: given a segment and the grep match end position, find the + // file argument. First try the token immediately after PAT; if none qualifies, + // try a cat/tac producer or < FILE redirect from the full segment. + const resolveFileArg = (segment: string, afterPatStr: string): string | null => { + // Primary: token immediately after PAT in the grep command + const fileM = afterPatStr.match(/^\s+([^\s'"|>&;$()\[\]]+)/); + const rawFile = fileM ? fileM[1] : null; + if (rawFile && isFileLike(rawFile)) return rawFile; + // FIX 4: For NON-region segments, also look for cat/tac producer or < FILE redirect + const catM = segment.match(/\b(?:cat|tac)\s+([^\s'"|>&;()]+)/); + if (catM && isFileLike(catM[1])) return catM[1]; + const redirM = segment.match(/<\s*([^\s'"|>&;()]+)/); + if (redirM && isFileLike(redirM[1])) return redirM[1]; + return null; + }; + + // If leading !, it might be a count or a direct !grep + if (hasLeadingNot && !hasCountZero) { + // Direct ! grep PAT FILE form: grep opts PAT FILE + // Extract PAT and FILE from the grep invocation + grepArgRe.lastIndex = 0; + let gm: RegExpExecArray | null; + while ((gm = grepArgRe.exec(seg)) !== null) { + const opts = gm[1]; + if (optsHaveInvert(opts)) continue; // -v form: not a ban + // PAT + const rawPat = gm[2] !== undefined ? gm[2] : + gm[3] !== undefined ? gm[3] : + gm[4] !== undefined && plausibleBare(gm[4]) ? gm[4] : null; + if (!rawPat) continue; + // FILE: next non-option token after PAT (or cat/tac/redirect in segment) + const afterPat = seg.slice((gm.index || 0) + gm[0].length); + const rawFile = resolveFileArg(seg, afterPat); + if (rawFile) { + pat = rawPat; + file = rawFile; + isBan = true; + } + } + } + + if (!isBan && hasCountZero) { + // count grep form: grep -c PAT FILE == 0 or [ $(grep -c PAT FILE) -eq 0 ] + grepArgRe.lastIndex = 0; + let gm: RegExpExecArray | null; + while ((gm = grepArgRe.exec(seg)) !== null) { + const opts = gm[1]; + if (!optsHaveCount(opts) || optsHaveInvert(opts)) continue; + const rawPat = gm[2] !== undefined ? gm[2] : + gm[3] !== undefined ? gm[3] : + gm[4] !== undefined && plausibleBare(gm[4]) ? gm[4] : null; + if (!rawPat) continue; + const afterPat = seg.slice((gm.index || 0) + gm[0].length); + const rawFile = resolveFileArg(seg, afterPat); + if (rawFile) { + pat = rawPat; + file = rawFile; + isBan = true; + } + } + } + + if (!isBan || !pat || !file) continue; + if (allow.has(pat)) continue; + // Skip if region-scoped (grep downstream of a sed/awk pipe — region extracted) + if (isRegionScoped(seg)) continue; + + // For each other task B: check if B's includes FILE AND B's reqText contains PAT + for (let bi = 0; bi < tasks.length; bi++) { + if (bi === ai) continue; + const taskB = tasks[bi]; + + // FIX 2: Exact normalized match; basename fallback ONLY for unqualified gate files. + const gateFile = normPath(file); + const bMatchesFile = taskB.files.some((bf) => { + const nbf = normPath(bf); + if (nbf === gateFile) return true; + // basename fallback only when the gate file is an unqualified bare filename (no dir separator) + if (!gateFile.includes('/') && path.basename(nbf) === gateFile) return true; + return false; + }); + if (!bMatchesFile) continue; + + // FIX 1: Use linear-time patternRequiredIn instead of new RegExp (ReDoS-safe). + const bRequiresPat = patternRequiredIn(pat, taskB.reqText); + if (!bRequiresPat) continue; + + const dedupeKey = `${ai}:${bi}:${pat}:${file}`; + if (seen.has(dedupeKey)) continue; + seen.add(dedupeKey); + + warnings.push( + `Region-scope conflict (#968): task "${taskA.name}" negative-greps "${pat}" file-wide on ${file}, ` + + `but sibling task "${taskB.name}" requires it in the same file. ` + + `A file-wide ban is unsatisfiable when a sibling needs the construct elsewhere — ` + + `region-scope task "${taskA.name}"'s gate (sed -n/awk range then grep) or use an AST/test check. ` + + `See planner-antipatterns.md "Region-Scoped Negative Gates", or add ` + + ` if intentional.`, + ); + } + } + } + + // This detector is warn-only: it never sets valid=false. + return { warnings, valid: true as const }; +} + function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): void { if (!filePath) { error('file path required'); @@ -315,6 +619,9 @@ function cmdVerifyPlanStructure(cwd: string, filePath: string, raw: boolean): vo errors.push(...echoScan.errors); warnings.push(...echoScan.warnings); + const conflictScan = scanFileWideNegativeGateConflict(content); + warnings.push(...conflictScan.warnings); + output( { valid: errors.length === 0, @@ -1935,6 +2242,7 @@ function cmdVerifyCodebaseDrift(cwd: string, raw: boolean): void { export = { scanNegativeGrepCommentEcho, + scanFileWideNegativeGateConflict, cmdVerifySummary, cmdVerifyPlanStructure, cmdVerifyPhaseCompleteness, diff --git a/tests/agent-size-baseline.json b/tests/agent-size-baseline.json index f6ec64c07..e320d007c 100644 --- a/tests/agent-size-baseline.json +++ b/tests/agent-size-baseline.json @@ -23,7 +23,7 @@ "gsd-pattern-mapper.md": 12487, "gsd-phase-researcher.md": 40638, "gsd-plan-checker.md": 42003, - "gsd-planner.md": 48892, + "gsd-planner.md": 49216, "gsd-project-researcher.md": 22014, "gsd-research-synthesizer.md": 13653, "gsd-roadmapper.md": 21781, diff --git a/tests/bug-641-files-from-suite-token.test.cjs b/tests/bug-641-files-from-suite-token.test.cjs index 25462e129..a427fc527 100644 --- a/tests/bug-641-files-from-suite-token.test.cjs +++ b/tests/bug-641-files-from-suite-token.test.cjs @@ -147,3 +147,103 @@ describe('bug #641 — --files-from with bare suite token', () => { assert.ok(r.stderr.includes('a.test.cjs'), `unit test must run.\nstderr: ${r.stderr}`); }); }); + +// Regression test for issue #1329: +// ci-prepare-test-scope's empty-detection FALLBACK hardcoded an explicit file +// list that included tests/core.test.cjs — a file deleted in #1291. Every +// scoped lane (scope=targeted|windows) that hit the fallback wrote the stale +// path into .ci-selected-tests.txt and crashed run-tests with +// "requested test file(s) not found: core.test.cjs". The fix: existence-filter +// the fallback at write time, fall back to the 'unit' suite sentinel when +// nothing survives, and guard the FALLBACK constant against disk reality. +describe('bug #1329 — ci-prepare-test-scope fallback never emits a deleted file', () => { + const { FALLBACK, FALLBACK_SENTINEL, SUITE_SENTINELS, resolveSelection } = + require('../scripts/ci-prepare-test-scope.cjs'); + const REPO_ROOT = path.join(__dirname, '..'); + + // Generative parity guard (DEFECT.GENERATIVE-FIX): the FALLBACK constant and + // the test files on disk are two surfaces that must stay in sync. This fails + // the instant a refactor deletes a file still named in FALLBACK — which is + // precisely what #1291 did and CI did not catch. + test('every FALLBACK entry resolves on disk or is a known suite sentinel', () => { + for (const entry of FALLBACK) { + const isSentinel = SUITE_SENTINELS.includes(entry); + const exists = fs.existsSync(path.join(REPO_ROOT, entry)); + assert.ok( + isSentinel || exists, + `FALLBACK entry "${entry}" is neither an existing test file nor a suite sentinel — stale reference will crash scoped CI lanes (see #1329).`, + ); + } + }); + + let tmpDir; + beforeEach(() => { + tmpDir = createTempDir('gsd-1329-fallback-'); + fs.mkdirSync(path.join(tmpDir, 'tests'), { recursive: true }); + }); + afterEach(() => { + cleanup(tmpDir); + }); + + test('empty detection drops a non-existent fallback entry instead of emitting it', () => { + // Create all but the last FALLBACK file under a controlled root, simulating + // a since-deleted test (the #1329 mechanism), independent of which files + // FALLBACK happens to name today. + const present = FALLBACK.slice(0, -1); + const absent = FALLBACK[FALLBACK.length - 1]; + for (const f of present) { + fs.writeFileSync(path.join(tmpDir, f), PASS_BODY, 'utf8'); + } + + const lines = resolveSelection({ scope: 'targeted', targeted: '', windows: '', root: tmpDir }); + + assert.ok(!lines.includes(absent), `absent file "${absent}" must be filtered out, got: ${lines.join(', ')}`); + for (const f of present) { + assert.ok(lines.includes(f), `present file "${f}" must survive, got: ${lines.join(', ')}`); + } + }); + + test('empty detection with no surviving fallback files falls back to the unit sentinel', () => { + // tmpDir/tests exists but contains none of the FALLBACK files. + const lines = resolveSelection({ scope: 'windows', targeted: '', windows: '', root: tmpDir }); + assert.deepStrictEqual(lines, [FALLBACK_SENTINEL]); + }); + + test('detected list passes through verbatim — files and suite sentinels preserved, not existence-filtered', () => { + // The detected list is already filtered by affected-tests-lib and may carry + // a suite sentinel; ci-prepare-test-scope must not touch it. + const lines = resolveSelection({ + scope: 'targeted', + targeted: 'tests/does-not-exist.test.cjs unit', + windows: '', + root: tmpDir, + }); + assert.deepStrictEqual(lines, ['tests/does-not-exist.test.cjs', 'unit']); + }); + + test('end-to-end: the real script writes a fallback list whose every entry resolves', () => { + // Run the real script (subprocess) with empty detection inside an isolated + // root that holds the FALLBACK files, then verify every line it wrote into + // .ci-selected-tests.txt resolves — the exact scoped-lane path that crashed + // in #1329. Hermetic: the temp root is removed by afterEach's cleanup(). + for (const f of FALLBACK) { + fs.writeFileSync(path.join(tmpDir, f), PASS_BODY, 'utf8'); + } + const prep = spawnSync( + process.execPath, + [path.join(REPO_ROOT, 'scripts', 'ci-prepare-test-scope.cjs')], + { cwd: tmpDir, env: { ...process.env, TEST_SCOPE: 'targeted', TARGETED_TESTS: '', WINDOWS_TESTS: '' }, encoding: 'utf8' }, + ); + assert.strictEqual(prep.status, 0, `prepare step failed: ${prep.stderr}`); + + const selected = fs.readFileSync(path.join(tmpDir, '.ci-selected-tests.txt'), 'utf8'); + for (const line of selected.split('\n').filter(Boolean)) { + const isSentinel = SUITE_SENTINELS.includes(line); + assert.ok( + isSentinel || fs.existsSync(path.join(tmpDir, line)), + `selected entry "${line}" does not resolve — would crash run-tests (#1329)`, + ); + } + assert.doesNotMatch(selected, /core\.test\.cjs/, 'deleted core.test.cjs must never be selected'); + }); +}); diff --git a/tests/capability-precedence-parity.test.cjs b/tests/capability-precedence-parity.test.cjs new file mode 100644 index 000000000..c6ece69aa --- /dev/null +++ b/tests/capability-precedence-parity.test.cjs @@ -0,0 +1,596 @@ +'use strict'; +/** + * capability-precedence-parity.test.cjs — DEFECT.GENERATIVE-FIX parity gate + * + * Proves that the config-key four-level precedence walk has a single owner + * (capability-activation.cjs) and that loop-resolver.cjs re-exports the same + * functions rather than maintaining local copies. + * + * Two contracts enforced: + * (a) Identity: loop-resolver's _resolveActivationValue, _getNestedConfigValue, + * and _readRawConfigKey are the SAME function objects as + * capability-activation's exports. Fails the instant a local copy is + * re-introduced. + * (b) Behavioral matrix: across a fixture matrix hitting each precedence level + * (loadConfig result, workstream config.json, root config.json, registry + * default, absent, falsy values, prototype-pollution key), asserts that + * _resolveActivationValue(key,config,cwd,registry) === + * (resolveConfigKey(key,{config,cwd,registry}).found ? + * Boolean(resolveConfigKey(key,{config,cwd,registry}).value) : false). + * Level-2/3 cases use real tmpdir .planning/config.json fixtures. + * + * RULESET: RULESET.TESTS.no-source-grep — no readFileSync + .includes() on source. + * RULESET: RULESET.TESTS.boundary-coverage — covers limit-1/limit/limit+1 (falsy boundary). + */ + +process.env.GSD_TEST_MODE = '1'; + +const { describe, test, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { cleanup } = require('./helpers.cjs'); + +// ── Module paths ────────────────────────────────────────────────────────────── + +const LIB = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib'); + +const capabilityActivation = require(path.join(LIB, 'capability-activation.cjs')); +const loopResolver = require(path.join(LIB, 'loop-resolver.cjs')); + +// ─── (a) Identity guard ─────────────────────────────────────────────────────── + +describe('capability-precedence-parity: identity guard — loop-resolver re-exports same function objects', () => { + test('_resolveActivationValue is the same function in both modules', () => { + assert.strictEqual( + loopResolver._resolveActivationValue, + capabilityActivation._resolveActivationValue, + '_resolveActivationValue: loop-resolver must re-export the capability-activation.cjs function, not a local copy', + ); + }); + + test('_getNestedConfigValue is the same function in both modules', () => { + assert.strictEqual( + loopResolver._getNestedConfigValue, + capabilityActivation._getNestedConfigValue, + '_getNestedConfigValue: loop-resolver must re-export the capability-activation.cjs function, not a local copy', + ); + }); + + test('_readRawConfigKey is the same function in both modules', () => { + assert.strictEqual( + loopResolver._readRawConfigKey, + capabilityActivation._readRawConfigKey, + '_readRawConfigKey: loop-resolver must re-export the capability-activation.cjs function, not a local copy', + ); + }); +}); + +// ─── (b) Behavioral matrix ──────────────────────────────────────────────────── + +/** + * Build a minimal registry with a configSchema entry for a dotted key. + * def=undefined means no default (absent). def= provides a default. + */ +function makeRegistry(dotKey, def) { + const registry = {}; + if (def !== undefined) { + registry.configSchema = { + [dotKey]: { default: def }, + }; + } else { + registry.configSchema = {}; + } + return registry; +} + +/** + * Helper: call resolveConfigKey and assert it equals + * _resolveActivationValue(key,config,cwd,registry) on the boolean coercion. + */ +function assertParity(key, config, cwd, registry, label) { + const { _resolveActivationValue, resolveConfigKey } = capabilityActivation; + const boolResult = _resolveActivationValue(key, config, cwd, registry); + const rawResult = resolveConfigKey(key, { config, cwd, registry }); + const expectedBool = rawResult.found ? Boolean(rawResult.value) : false; + assert.strictEqual( + boolResult, + expectedBool, + `parity check failed for ${label}: _resolveActivationValue=${boolResult} but resolveConfigKey gives found=${rawResult.found} value=${JSON.stringify(rawResult.value)} → expectedBool=${expectedBool}`, + ); + return { boolResult, rawResult }; +} + +describe('capability-precedence-parity: behavioral matrix', () => { + let tmpDir; + + before(() => { + // Create a hermetic tmpdir for file-based fixtures (levels 2+3). + // Clear env vars that redirect planningDir/planningRoot. + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'parity-test-')); + }); + + after(() => { + // Remove the tmpdir (helpers.cleanup carries the Windows-EBUSY retry budget). + cleanup(tmpDir); + }); + + // Env vars that planningDir/planningRoot read — save/restore around each test. + function withCleanEnv(fn) { + const saved = { + GSD_WORKSTREAM: process.env.GSD_WORKSTREAM, + GSD_PROJECT: process.env.GSD_PROJECT, + CLAUDE_CONFIG_DIR: process.env.CLAUDE_CONFIG_DIR, + }; + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; + delete process.env.CLAUDE_CONFIG_DIR; + try { + return fn(); + } finally { + if (saved.GSD_WORKSTREAM !== undefined) process.env.GSD_WORKSTREAM = saved.GSD_WORKSTREAM; + else delete process.env.GSD_WORKSTREAM; + if (saved.GSD_PROJECT !== undefined) process.env.GSD_PROJECT = saved.GSD_PROJECT; + else delete process.env.GSD_PROJECT; + if (saved.CLAUDE_CONFIG_DIR !== undefined) process.env.CLAUDE_CONFIG_DIR = saved.CLAUDE_CONFIG_DIR; + else delete process.env.CLAUDE_CONFIG_DIR; + } + } + + // ── Level 1: loadConfig result (config arg has the key) ───────────────────── + + test('level-1 truthy: config arg has key=true → resolved true, parity holds', () => { + const key = 'feature.enabled'; + const config = { feature: { enabled: true } }; + const registry = makeRegistry(key, undefined); + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'level-1 truthy'); + assert.strictEqual(boolResult, true); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, true); + }); + + test('level-1 falsy (false): config arg has key=false → resolved false, parity holds', () => { + // BVA: falsy value false — boundary case + const key = 'feature.enabled'; + const config = { feature: { enabled: false } }; + const registry = makeRegistry(key, true); // default true but level-1 wins + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'level-1 falsy=false'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, false); // raw value preserved + }); + + test('level-1 falsy (0): config arg has key=0 → resolved false, parity holds', () => { + // BVA: falsy value 0 — boundary case + const key = 'feature.level'; + const config = { feature: { level: 0 } }; + const registry = makeRegistry(key, 2); // default 2 but level-1 wins + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'level-1 falsy=0'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, 0); // raw value 0 preserved + }); + + test('level-1 truthy (nonzero): config arg has key=2 → resolved true, parity holds', () => { + // BVA: truthy numeric — boundary-adjacent to 0 + const key = 'feature.level'; + const config = { feature: { level: 2 } }; + const registry = makeRegistry(key, undefined); + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'level-1 truthy=2'); + assert.strictEqual(boolResult, true); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, 2); + }); + + // ── Level 2: workstream config.json ───────────────────────────────────────── + + test('level-2: workstream config.json has key → resolved, level-1 empty, parity holds', () => { + withCleanEnv(() => { + // Set up a .planning/config.json in tmpDir + const planningDir = path.join(tmpDir, 'ws-l2', '.planning'); + fs.mkdirSync(planningDir, { recursive: true }); + fs.writeFileSync( + path.join(planningDir, 'config.json'), + JSON.stringify({ graphify: { enabled: true } }), + 'utf8', + ); + const key = 'graphify.enabled'; + const config = {}; // level-1 miss + const registry = makeRegistry(key, undefined); + const cwd = path.join(tmpDir, 'ws-l2'); + const { boolResult, rawResult } = assertParity(key, config, cwd, registry, 'level-2 workstream'); + assert.strictEqual(boolResult, true); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, true); + }); + }); + + test('level-2 falsy: workstream config.json has key=false → false, parity holds', () => { + withCleanEnv(() => { + const planningDir = path.join(tmpDir, 'ws-l2-false', '.planning'); + fs.mkdirSync(planningDir, { recursive: true }); + fs.writeFileSync( + path.join(planningDir, 'config.json'), + JSON.stringify({ graphify: { enabled: false } }), + 'utf8', + ); + const key = 'graphify.enabled'; + const config = {}; + const registry = makeRegistry(key, true); // default true, but level-2 wins + const cwd = path.join(tmpDir, 'ws-l2-false'); + const { boolResult, rawResult } = assertParity(key, config, cwd, registry, 'level-2 falsy'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, false); + }); + }); + + // ── Level 3: root config.json (only when paths differ) ────────────────────── + + // Without GSD_WORKSTREAM, planningDir === planningRoot, so levels 2 and 3 point + // to the same file. Level-3 distinction is only exercisable by setting GSD_WORKSTREAM. + // We test the "paths same → no double-read" path (covered by level-2 test above) + // and the "paths differ" path via GSD_WORKSTREAM. + test('level-3: root config.json read when workstream path differs, parity holds', () => { + // Use GSD_WORKSTREAM to force planningDir to differ from planningRoot. + withCleanEnv(() => { + const projectRoot = path.join(tmpDir, 'ws-l3-project'); + // Root planning dir = projectRoot/.planning + const rootPlanningDir = path.join(projectRoot, '.planning'); + // Workstream planning dir = projectRoot/.planning/workstreams/mystream + const wsName = 'mystream'; + const wsPlanningDir = path.join(rootPlanningDir, 'workstreams', wsName); + fs.mkdirSync(rootPlanningDir, { recursive: true }); + fs.mkdirSync(wsPlanningDir, { recursive: true }); + + // Write ONLY the root config.json (no ws config.json) + fs.writeFileSync( + path.join(rootPlanningDir, 'config.json'), + JSON.stringify({ intel: { enabled: true } }), + 'utf8', + ); + + // Activate workstream so planningDir → wsPlanningDir, planningRoot → rootPlanningDir + process.env.GSD_WORKSTREAM = wsName; + + const key = 'intel.enabled'; + const config = {}; + const registry = makeRegistry(key, undefined); + const { boolResult, rawResult } = assertParity(key, config, projectRoot, registry, 'level-3 root config'); + // ws config.json absent → level-2 miss; root config.json present → level-3 hit + assert.strictEqual(boolResult, true); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, true); + }); + }); + + // ── Level 4: registry configSchema default ─────────────────────────────────── + + test('level-4: registry default=true, no config or file → true, parity holds', () => { + withCleanEnv(() => { + const key = 'workflow.some_feature'; + const config = {}; + const registry = makeRegistry(key, true); + // cwd points to an empty dir — no .planning/config.json files + const cwd = path.join(tmpDir, 'l4-default-true'); + fs.mkdirSync(cwd, { recursive: true }); + const { boolResult, rawResult } = assertParity(key, config, cwd, registry, 'level-4 default=true'); + assert.strictEqual(boolResult, true); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, true); + }); + }); + + test('level-4: registry default=false → false (BVA: falsy default), parity holds', () => { + withCleanEnv(() => { + const key = 'workflow.some_feature'; + const config = {}; + const registry = makeRegistry(key, false); + const cwd = path.join(tmpDir, 'l4-default-false'); + fs.mkdirSync(cwd, { recursive: true }); + const { boolResult, rawResult } = assertParity(key, config, cwd, registry, 'level-4 default=false'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, false); // raw false preserved + }); + }); + + test('level-4: registry default=0 → false (BVA: numeric zero default), parity holds', () => { + withCleanEnv(() => { + const key = 'feature.level'; + const config = {}; + const registry = makeRegistry(key, 0); + const cwd = path.join(tmpDir, 'l4-default-0'); + fs.mkdirSync(cwd, { recursive: true }); + const { boolResult, rawResult } = assertParity(key, config, cwd, registry, 'level-4 default=0'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, 0); // raw 0 preserved + }); + }); + + // ── Level 5: absent ────────────────────────────────────────────────────────── + + test('level-5: key absent everywhere → false, found=false, parity holds', () => { + withCleanEnv(() => { + const key = 'nonexistent.key'; + const config = {}; + const registry = makeRegistry(key, undefined); // no default + const cwd = path.join(tmpDir, 'l5-absent'); + fs.mkdirSync(cwd, { recursive: true }); + const { boolResult, rawResult } = assertParity(key, config, cwd, registry, 'level-5 absent'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, false); + assert.strictEqual(rawResult.value, undefined); + }); + }); + + test('level-5: cwd=undefined, absent key → false, found=false, parity holds', () => { + const key = 'nonexistent.key'; + const config = {}; + const registry = makeRegistry(key, undefined); + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'level-5 absent cwd=undefined'); + assert.strictEqual(boolResult, false); + assert.strictEqual(rawResult.found, false); + }); + + // ── Prototype-pollution guard (nested-traversal sink, levels 1–3) ────────────── + // + // The guard protects the NESTED config-object traversal (_getNestedConfigValue) — + // the only prototype-pollution sink. A `__proto__`/`constructor`/`prototype` + // SEGMENT encountered mid-path is rejected → found=false. Level 4 is a flat + // single-key lookup of the whole dotted string against registry.configSchema (no + // nested traversal, not a pollution sink), so we deliberately seed NO level-4 + // default here — otherwise that flat lookup would legitimately match the literal + // dotted key and the segment guard would not be the thing under test. + + test('prototype-pollution: __proto__ mid-path segment → found=false, boolResult=false, parity holds', () => { + const key = 'a.__proto__.polluted'; + const config = { a: { real: 1 } }; // 'a' resolves, then the '__proto__' segment must be rejected + const registry = makeRegistry(key, undefined); // no level-4 default + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'proto-pollution __proto__ mid-path'); + assert.strictEqual(rawResult.found, false, '__proto__ path segment must be rejected by the nested-traversal guard'); + assert.strictEqual(boolResult, false); + }); + + test('prototype-pollution: constructor mid-path segment → found=false, boolResult=false, parity holds', () => { + const key = 'a.constructor.polluted'; + const config = { a: { real: 1 } }; + const registry = makeRegistry(key, undefined); // no level-4 default + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'proto-pollution constructor mid-path'); + assert.strictEqual(rawResult.found, false, 'constructor path segment must be rejected by the nested-traversal guard'); + assert.strictEqual(boolResult, false); + }); + + // ── Level-1 precedence over level-4 ───────────────────────────────────────── + + test('level-1 wins over level-4 default: config=false beats registry default=true', () => { + // BVA: this is the critical boundary — level-1 falsy beats level-4 truthy + const key = 'graphify.enabled'; + const config = { graphify: { enabled: false } }; + const registry = makeRegistry(key, true); + const { boolResult, rawResult } = assertParity(key, config, undefined, registry, 'level-1 false beats level-4 true default'); + assert.strictEqual(boolResult, false, 'config value false must win over registry default true'); + assert.strictEqual(rawResult.found, true); + assert.strictEqual(rawResult.value, false); + }); +}); + +// ─── (c) resolveConfigKey identity guard (FIX 2 — loop-resolver re-exports resolveConfigKey) ──── +// +// resolveLoopHooks.resolveConfigValues uses resolveConfigKey internally to build +// hook configValues. Exporting resolveConfigKey from loop-resolver and asserting +// identity here ensures that if someone ever introduces a local copy, this test +// will catch it immediately (same mechanism as the _resolveActivationValue guard +// above, applied to the raw-value consumer). + +describe('capability-precedence-parity: identity guard — loop-resolver.resolveConfigKey is same fn as capability-activation.resolveConfigKey', () => { + test('resolveConfigKey is the same function in both modules', () => { + assert.ok( + typeof loopResolver.resolveConfigKey === 'function', + 'loop-resolver must export resolveConfigKey', + ); + assert.strictEqual( + loopResolver.resolveConfigKey, + capabilityActivation.resolveConfigKey, + 'resolveConfigKey: loop-resolver must re-export the capability-activation.cjs function, not a local copy', + ); + }); +}); + +// ─── (d) Behavioral: loop-resolver resolveConfigValues uses the shared engine ──────────────────── +// +// Proves that resolveLoopHooks's resolveConfigValues closure (the RAW-value consumer) +// produces the SAME result as calling capabilityActivation.resolveConfigKey directly +// for the same (dotKey, config, cwd, registry) arguments. +// +// This test FAILS if resolveConfigValues is ever replaced with a divergent copy that +// does not delegate to resolveConfigKey — fulfilling the DEFECT.GENERATIVE-FIX mandate. +// +// Coverage: three cases required: +// (1) Level-1 config hit — key present in config arg +// (2) Level-4 registry default — key absent from config, present in schema default +// (3) Absent key — key absent everywhere (omitted from resolved output) + +describe('capability-precedence-parity: behavioral — loop-resolver resolveConfigValues uses shared resolveConfigKey engine', () => { + const { resolveLoopHooks } = loopResolver; + const { resolveConfigKey } = capabilityActivation; + const { CANONICAL_POINTS_FALLBACK } = loopResolver; + + /** + * Build a minimal registry that has: + * - one contribution hook at the given point with configValues map + * - one capability that owns that hook (capId = 'test-cap', active=true implied) + * - configSchema entries for level-4 defaults + */ + function makeLoopRegistry({ point, configValues, schemaDefaults = {} }) { + const byLoopPoint = {}; + for (const p of CANONICAL_POINTS_FALLBACK) { + byLoopPoint[p] = { steps: [], contributions: [], gates: [] }; + } + byLoopPoint[point].contributions = [ + { + capId: 'test-cap', + into: 'test-section', + fragment: { inline: 'test fragment' }, + configValues, + }, + ]; + const configSchema = {}; + for (const [dotKey, def] of Object.entries(schemaDefaults)) { + configSchema[dotKey] = { default: def }; + } + return { byLoopPoint, configSchema }; + } + + /** capabilityStatesById with test-cap active=true */ + function activeCapMap() { + return new Map([['test-cap', { enabled: true, active: true }]]); + } + + test('(1) level-1 config hit: resolveConfigValues matches resolveConfigKey for present key', () => { + // Hook declares configValues: { secLevel: 'security.asvs_level' } + // Config has security.asvs_level=2 → level-1 hit + const dotKey = 'security.asvs_level'; + const alias = 'secLevel'; + const config = { security: { asvs_level: 2 } }; + const registry = makeLoopRegistry({ + point: 'plan:pre', + configValues: { [alias]: dotKey }, + }); + + const resolved = resolveLoopHooks({ + point: 'plan:pre', + registry, + config, + cwd: undefined, + capabilityStatesById: activeCapMap(), + }); + + assert.strictEqual(resolved.activeHooks.length, 1, 'One active contribution hook expected'); + const hook = resolved.activeHooks[0]; + assert.ok(hook.configValues, 'configValues must be present on the resolved hook'); + + // resolveConfigKey direct result for the same args + const directResult = resolveConfigKey(dotKey, { config, cwd: undefined, registry }); + assert.strictEqual(directResult.found, true, 'direct resolveConfigKey must find the level-1 key'); + assert.strictEqual( + hook.configValues[alias], + directResult.value, + `resolved hook configValues[${alias}] must equal resolveConfigKey(...).value for level-1 hit (both=${JSON.stringify(directResult.value)})`, + ); + assert.strictEqual(hook.configValues[alias], 2, 'level-1 numeric value 2 must be preserved (not coerced to boolean)'); + }); + + test('(2) level-4 registry default hit: resolveConfigValues matches resolveConfigKey for schema default', () => { + // Hook declares configValues: { blockOn: 'security.block_on' } + // Config is empty; schema default = 'medium' + const dotKey = 'security.block_on'; + const alias = 'blockOn'; + const schemaDefault = 'medium'; + const config = {}; + const registry = makeLoopRegistry({ + point: 'execute:pre', + configValues: { [alias]: dotKey }, + schemaDefaults: { [dotKey]: schemaDefault }, + }); + + const resolved = resolveLoopHooks({ + point: 'execute:pre', + registry, + config, + cwd: undefined, + capabilityStatesById: activeCapMap(), + }); + + assert.strictEqual(resolved.activeHooks.length, 1, 'One active contribution hook expected'); + const hook = resolved.activeHooks[0]; + assert.ok(hook.configValues, 'configValues must be present on the resolved hook (schema default found)'); + + const directResult = resolveConfigKey(dotKey, { config, cwd: undefined, registry }); + assert.strictEqual(directResult.found, true, 'direct resolveConfigKey must find the level-4 schema default'); + assert.strictEqual( + hook.configValues[alias], + directResult.value, + `resolved hook configValues[${alias}] must equal resolveConfigKey(...).value for level-4 hit (both=${JSON.stringify(directResult.value)})`, + ); + assert.strictEqual(hook.configValues[alias], 'medium', 'level-4 string default must be preserved as raw string'); + }); + + test('(3a) mixed hook: one resolvable alias + one absent alias → configValues present, contains only the resolvable alias', () => { + // Hook declares configValues: { present: 'feature.on', absent: 'nope.missing' } + // Config has feature.on=true (level-1 hit); no registry default for nope.missing. + // Expected: resolved hook.configValues is defined, has 'present'=true, does NOT have 'absent'. + const presentDotKey = 'feature.on'; + const absentDotKey = 'nope.missing'; + const config = { feature: { on: true } }; + const registry = makeLoopRegistry({ + point: 'verify:post', + configValues: { present: presentDotKey, absent: absentDotKey }, + // No schema default for absentDotKey — it must be absent everywhere + }); + + const resolved = resolveLoopHooks({ + point: 'verify:post', + registry, + config, + cwd: undefined, + capabilityStatesById: activeCapMap(), + }); + + assert.strictEqual(resolved.activeHooks.length, 1, 'One active contribution hook expected'); + const hook = resolved.activeHooks[0]; + + // The 'present' alias must resolve via level-1 config hit + assert.ok(hook.configValues !== undefined, 'configValues must be defined (at least one alias resolved)'); + assert.strictEqual( + hook.configValues['present'], + true, + "hook.configValues['present'] must equal true (level-1 hit for feature.on)", + ); + + // The 'absent' alias must NOT appear in configValues at all (omit-when-absent contract) + assert.ok( + !Object.prototype.hasOwnProperty.call(hook.configValues, 'absent'), + "Absent key 'nope.missing' must not appear in hook.configValues (found=false → omit, not include as undefined)", + ); + }); + + test('(3b) all-absent hook: all configValues aliases absent everywhere → hook.configValues === undefined', () => { + // Hook declares configValues: { missingAlias: 'does.not.exist' } + // Key absent from config, no schema default, no config files. + // resolveConfigValues must return undefined (empty resolved map → omitted entirely). + const dotKey = 'does.not.exist'; + const alias = 'missingAlias'; + const config = {}; + const registry = makeLoopRegistry({ + point: 'ship:pre', + configValues: { [alias]: dotKey }, + // No schema default for this key + }); + + const resolved = resolveLoopHooks({ + point: 'ship:pre', + registry, + config, + cwd: undefined, + capabilityStatesById: activeCapMap(), + }); + + assert.strictEqual(resolved.activeHooks.length, 1, 'One active contribution hook expected'); + const hook = resolved.activeHooks[0]; + + // resolveConfigKey direct result must confirm absent + const directResult = resolveConfigKey(dotKey, { config, cwd: undefined, registry }); + assert.strictEqual(directResult.found, false, 'resolveConfigKey must return found=false for absent key'); + assert.strictEqual(directResult.value, undefined); + + // When ALL aliases are absent, resolveConfigValues returns undefined → hook.configValues must be undefined + assert.strictEqual( + hook.configValues, + undefined, + 'hook.configValues must be undefined when all aliases are absent (omit-when-empty contract)', + ); + }); +}); diff --git a/tests/capability-state.test.cjs b/tests/capability-state.test.cjs index 21d2feb87..2656819db 100644 --- a/tests/capability-state.test.cjs +++ b/tests/capability-state.test.cjs @@ -1461,6 +1461,136 @@ describe('isCapabilityActive — convenience predicate (Phase 2)', () => { }); }); +// ─── resolveCapabilityRuntimeState configOverride (FIX 1 — single config snapshot) ────────────── + +describe('resolveCapabilityRuntimeState configOverride — honors caller snapshot instead of reading disk', () => { + // These tests exercise the optional third parameter `configOverride`. + // When provided, the resolver MUST use that config object for capability + // activation (active = enabled && configActivation) rather than calling + // loadConfig(cwd) internally. This guarantees that cmdLoopRenderHooks and + // the resolver use the exact same config snapshot (TOCTOU elimination). + // + // Strategy: build a synthetic registry with `activationKey: 'myfeature.enabled'` + // and use a tmpdir whose on-disk .planning/config.json carries the OPPOSITE + // value from the configOverride. The test asserts that the returned capability + // `active` matches the override, not the on-disk file. + + const { resolveCapabilityRuntimeState } = require('../gsd-core/bin/lib/capability-state.cjs'); + + let tmpDir; + let savedEnv; + + before(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-state-override-')); + // Save env vars that affect planningDir / config resolution + savedEnv = { + GSD_WORKSTREAM: process.env.GSD_WORKSTREAM, + GSD_PROJECT: process.env.GSD_PROJECT, + CLAUDE_CONFIG_DIR: process.env.CLAUDE_CONFIG_DIR, + }; + delete process.env.GSD_WORKSTREAM; + delete process.env.GSD_PROJECT; + delete process.env.CLAUDE_CONFIG_DIR; + + // Write on-disk config with myfeature.enabled=false (override will say true) + const planningDir = path.join(tmpDir, '.planning'); + fs.mkdirSync(planningDir, { recursive: true }); + fs.writeFileSync( + path.join(planningDir, 'config.json'), + JSON.stringify({ myfeature: { enabled: false } }), + 'utf8', + ); + }); + + after(() => { + // Restore env vars + if (savedEnv.GSD_WORKSTREAM !== undefined) process.env.GSD_WORKSTREAM = savedEnv.GSD_WORKSTREAM; + else delete process.env.GSD_WORKSTREAM; + if (savedEnv.GSD_PROJECT !== undefined) process.env.GSD_PROJECT = savedEnv.GSD_PROJECT; + else delete process.env.GSD_PROJECT; + if (savedEnv.CLAUDE_CONFIG_DIR !== undefined) process.env.CLAUDE_CONFIG_DIR = savedEnv.CLAUDE_CONFIG_DIR; + else delete process.env.CLAUDE_CONFIG_DIR; + cleanup(tmpDir); + }); + + test('configOverride=undefined (default) reads on-disk config → active=false', () => { + // Baseline: without override, loadConfig reads the on-disk file which has + // myfeature.enabled=false, so the capability is inactive. + // NOTE: resolveCapabilityRuntimeState does real I/O (profile/surface), which + // degrades gracefully on a tmpDir that has no surface marker. We only assert + // on the `active` field which is determined by the config activation leg. + // Use a simple registry with no skills so installed=surfaced=enabled=true + // (vacuously) and active depends solely on configActivation. + // + // Because resolveCapabilityRuntimeState always uses the real capability-registry.cjs, + // we cannot inject the synthetic registry into it. Instead we verify the + // config-override leg by comparing the two code paths (with and without override) + // against each other on a known config key that the real registry uses. + // + // We use the real 'graphify' capability whose activationKey='graphify.enabled'. + // On-disk: graphify.enabled=false → active=false. + // Override: { graphify: { enabled: true } } → active=true. + // + // Set up a separate tmpdir with graphify.enabled=false on disk. + const tmpCwd = fs.mkdtempSync(path.join(os.tmpdir(), 'cap-override-baseline-')); + try { + fs.mkdirSync(path.join(tmpCwd, '.planning'), { recursive: true }); + fs.writeFileSync( + path.join(tmpCwd, '.planning', 'config.json'), + JSON.stringify({ graphify: { enabled: false } }), + 'utf8', + ); + // No override → reads on-disk config → graphify active=false + const result = resolveCapabilityRuntimeState(tmpCwd, undefined); + const graphifyEntry = result.capabilities.find((c) => c.id === 'graphify'); + assert.ok(graphifyEntry, 'graphify capability must be present in the real registry'); + assert.strictEqual( + graphifyEntry.active, + false, + 'Without configOverride, on-disk graphify.enabled=false must produce active=false', + ); + } finally { + cleanup(tmpCwd); + } + }); + + test('configOverride honors caller value — overrides on-disk config (active flips from false to true)', () => { + // On-disk: graphify.enabled=false (set up in before()). + // configOverride: { graphify: { enabled: true } } ← caller wins. + // Expected: graphify active=true (override, not disk). + // + // The tmpDir from before() has graphify.enabled=false on disk. We pass + // configOverride with graphify.enabled=true. The returned active must be true. + const override = { graphify: { enabled: true } }; + const result = resolveCapabilityRuntimeState(tmpDir, undefined, override); + const graphifyEntry = result.capabilities.find((c) => c.id === 'graphify'); + assert.ok(graphifyEntry, 'graphify capability must be present in the real registry'); + assert.strictEqual( + graphifyEntry.active, + true, + 'configOverride { graphify: { enabled: true } } must produce active=true even though on-disk config has graphify.enabled=false', + ); + }); + + test('configOverride — non-existent cwd, override with enabled=true → active=true regardless of absent disk file', () => { + // When cwd points to a non-existent directory, loadConfig(cwd) would return {} + // (or throw/fallback to {}). With configOverride, the caller's value is used + // for level-1 directly. Levels 2+3 raw reads on a non-existent cwd produce + // no-hits. So: override={graphify:{enabled:true}} → level-1 hit → active=true. + // This is a clean test that avoids any interaction with the on-disk fixture in tmpDir. + const nonExistentCwd = path.join(os.tmpdir(), 'cap-override-nonexistent-' + Date.now()); + const override = { graphify: { enabled: true } }; + const result = resolveCapabilityRuntimeState(nonExistentCwd, undefined, override); + const graphifyEntry = result.capabilities.find((c) => c.id === 'graphify'); + assert.ok(graphifyEntry, 'graphify capability must be present in the real registry'); + assert.strictEqual( + graphifyEntry.active, + true, + 'configOverride { graphify: { enabled: true } } must produce active=true even with non-existent cwd (level-1 hit from override)', + ); + }); +}); + // ─── Cross-runtime runtime detection (HIGH fix — GSD_RUNTIME → config.runtime → 'claude') ────── describe('isCapabilityActive cross-runtime detection (GSD_RUNTIME → config.runtime → claude)', () => { diff --git a/tests/enh-968-region-scoped-negative-grep.test.cjs b/tests/enh-968-region-scoped-negative-grep.test.cjs new file mode 100644 index 000000000..5e76a4650 --- /dev/null +++ b/tests/enh-968-region-scoped-negative-grep.test.cjs @@ -0,0 +1,823 @@ +// allow-test-rule: source-text-is-the-product #968 +// Enhancement #968: region-scoped negative gate detector + guidance docs. +// Tests the pure function scanFileWideNegativeGateConflict exported from +// verify.cjs, plus CLI integration and doc-contract assertions. + +'use strict'; + +const { test, describe, before, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); +const os = require('node:os'); +const { createTempProject, cleanup, runGsdTools } = require('./helpers.cjs'); + +// Build path to built verify.cjs +const VERIFY_CJS = path.join(__dirname, '..', 'gsd-core', 'bin', 'lib', 'verify.cjs'); + +// Build paths to doc files +const PLANNER_MD = path.join(__dirname, '..', 'agents', 'gsd-planner.md'); +const ANTIPATTERNS_MD = path.join(__dirname, '..', 'gsd-core', 'references', 'planner-antipatterns.md'); +// PLAN_MD_REF removed — was unused (doc-contract cases test PLANNER_MD and ANTIPATTERNS_MD only) + +// ─── Fixture helpers ─────────────────────────────────────────────────────────── + +/** + * Build a minimal two-task plan fixture. + * taskA: file=app/page.py, gateText=the verify/acceptance_criteria block, actionText=action block + * taskB: file=app/page.py (default) or otherFile, action text + * allowlistMarker: optional HTML comment to insert at the top + */ +function makeTwoTaskPlan({ + taskAFile = 'app/page.py', + taskAGate = '! grep -Eq \'await .*refresh\' app/page.py', + taskAAction = 'Refactor the factory to be synchronous.', + taskBFile = 'app/page.py', + taskBAction = 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + allowlistMarker = '', +} = {}) { + const lines = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + `files_modified: [${taskAFile}, ${taskBFile}]`, + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '# Test Plan', + '', + ]; + + if (allowlistMarker) { + lines.push(allowlistMarker, ''); + } + + // Task A: the one with the negative gate + lines.push(''); + lines.push('Task A: factory refactor'); + lines.push(`${taskAFile}`); + lines.push(`${taskAAction}`); + lines.push(`${taskAGate}`); + lines.push('Factory is synchronous.'); + lines.push(''); + lines.push(''); + + // Task B: the sibling that requires the construct + lines.push(''); + lines.push('Task B: reindex handler'); + lines.push(`${taskBFile}`); + lines.push(`${taskBAction}`); + lines.push('npm test'); + lines.push('Handler is in place.'); + lines.push(''); + + return lines.join('\n'); +} + +/** + * Build a single-task plan (no sibling). + */ +function makeSingleTaskPlan({ + taskFile = 'app/page.py', + taskGate = '! grep -Eq \'await .*refresh\' app/page.py', + taskAction = 'Refactor the factory to be synchronous.', +} = {}) { + return [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + `files_modified: [${taskFile}]`, + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '# Test Plan', + '', + '', + 'Task A: factory refactor', + `${taskFile}`, + `${taskAction}`, + `${taskGate}`, + 'Factory is synchronous.', + '', + ].join('\n'); +} + +// ─── Group 1: pure-function unit tests ──────────────────────────────────────── + +describe('scanFileWideNegativeGateConflict — pure unit tests', () => { + let scan; + + before(() => { + const verify = require(VERIFY_CJS); + scan = verify.scanFileWideNegativeGateConflict; + assert.ok(typeof scan === 'function', 'scanFileWideNegativeGateConflict must be exported'); + }); + + // Case 1: basic WARN path — Task A bans PAT file-wide, Task B requires it in same file + test('case 1 — file-wide ban + sibling requires → WARN', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok(Array.isArray(result.warnings), 'must return { warnings: [] }'); + assert.ok( + result.warnings.length >= 1, + `expected at least 1 warning, got: ${JSON.stringify(result.warnings)}`, + ); + assert.ok( + result.warnings[0].includes('Region-scope conflict (#968)'), + `warning must mention Region-scope conflict (#968), got: ${result.warnings[0]}`, + ); + assert.ok( + result.warnings[0].includes('await .*refresh'), + `warning must mention the PAT, got: ${result.warnings[0]}`, + ); + assert.ok( + result.warnings[0].includes('app/page.py'), + `warning must mention the file, got: ${result.warnings[0]}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 2: region-scoped via sed → NO warn + test('case 2 — region-scoped via sed pipe → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! sed -n '12,40p' app/page.py | grep -Eq 'await .*refresh'", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `sed-piped grep is region-scoped — must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 2b: region-scoped via awk → NO warn + test('case 2b — region-scoped via awk pipe → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! awk '/^def make_page/,/^def /' app/page.py | grep -Eq 'await .*refresh'", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `awk-piped grep is region-scoped — must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 3: single task file-wide ban, no sibling → NO warn + test('case 3 — single task, no sibling → NO warn', () => { + const content = makeSingleTaskPlan({ + taskGate: "! grep -Eq 'await .*refresh' app/page.py", + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `single task (no sibling) must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 4: sibling requires PAT but lists a different file → NO warn + test('case 4 — sibling lists different file → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAFile: 'app/page.py', + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBFile: 'app/other.py', + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `sibling with different file must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 5: sibling lists same file but action lacks PAT → NO warn + test('case 5 — sibling lists same file but action lacks PAT → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls bridge.sync() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `sibling with no PAT in action must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 6: positive grep (no !) + sibling → NO warn (positive requirement, not a ban) + test('case 6 — positive grep (no !) + sibling → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "grep -q 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `positive grep must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 7: inverted grep -v with ! + sibling → NO warn + test('case 7 — inverted grep -vq with ! + sibling → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -vq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `inverted grep (-v) must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 8: allowlist marker present → NO warn + test('case 8 — allowlist marker suppresses warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + allowlistMarker: '', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `allowlist marker must suppress warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 9: one task both bans and requires same PAT in same file (no second task) → NO warn + test('case 9 — one task bans and requires PAT (no sibling) → NO warn', () => { + const content = makeSingleTaskPlan({ + taskGate: "! grep -Eq 'await .*refresh' app/page.py", + taskAction: 'Refactor to avoid await refresh, but note that bridge.refresh() is used later.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `single task (no sibling B) must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 10: count form `grep -c 'PAT' FILE == 0` + sibling → WARN + test('case 10 — count form (grep -c PAT FILE == 0) + sibling → WARN', () => { + const content = makeTwoTaskPlan({ + taskAGate: "grep -c 'await .*refresh' app/page.py == 0", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `count form (grep -c ... == 0) must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 11: bracket form `[ $(grep -c PAT FILE) -eq 0 ]` + sibling → WARN + test('case 11 — bracket form ([ $(grep -c PAT FILE) -eq 0 ]) + sibling → WARN', () => { + const content = makeTwoTaskPlan({ + taskAGate: "[ $(grep -c 'await .*refresh' app/page.py) -eq 0 ]", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `bracket form must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 12: CRLF variant of case 1 → WARN + test('case 12 — CRLF line endings → WARN', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const crlfContent = content.split('\n').join('\r\n'); + const result = scan(crlfContent); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `CRLF content must still warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 13: backslash line-continuation variant → WARN + test('case 13 — backslash line continuation → WARN', () => { + // Build manually to control exact line continuation + const lines = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: [app/page.py]', + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '', + 'Task A: factory refactor', + 'app/page.py', + 'Refactor the factory to be synchronous.', + // Gate split across lines with backslash continuation + "! grep -Eq 'await .*refresh' \\\napp/page.py", + 'Factory is synchronous.', + '', + '', + '', + 'Task B: reindex handler', + 'app/page.py', + 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + 'npm test', + 'Handler is in place.', + '', + ].join('\n'); + const result = scan(lines); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `backslash continuation must still warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 14: mixed line with positive gate AND a negative gate, sibling → WARN (on the negative only) + test('case 14 — mixed positive+negative on one segment + sibling → WARN for negative', () => { + const content = makeTwoTaskPlan({ + taskAGate: "grep -c 'X' app/page.py == 1 && ! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `mixed line with negative gate + sibling must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 15: glob file arg `app/*.py` → NO warn (unresolvable path) + test('case 15 — glob file arg → NO warn (unresolvable)', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/*.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `glob file arg must not warn (unresolvable), got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 16: invalid-regex PAT literal fallback → WARN, no exception + test('case 16 — invalid-regex PAT → literal fallback, WARN, no exception', () => { + // "await (refresh" — unbalanced paren, invalid regex + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await (refresh' app/page.py", + taskBAction: 'The handler calls await (refresh on bridge to repopulate state.', + }); + let result; + assert.doesNotThrow(() => { + result = scan(content); + }, 'scan must not throw on invalid regex PAT'); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `invalid-regex PAT with literal match must warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 17: ReDoS-ish PAT (catastrophic backtracking) → no hang, no false warn. + // The sibling action is 5000 'a's — classic ReDoS trigger if we call new RegExp('(a+)+$'). + // Proof-of-no-hang: the test runner's own timeout catches it; a hanging test fails here. + // No timing assertion (flaky) — the linear patternRequiredIn implementation is microsecond-fast. + test('case 17 — catastrophic ReDoS pattern is instant, no hang, no false warn', () => { + const longAs = 'a'.repeat(5000); + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq '(a+)+$' app/page.py", + taskBAction: `Reindex handler that processes ${longAs} records and calls bridge.refresh().`, + }); + let result; + assert.doesNotThrow(() => { + result = scan(content); + }, 'scan must not throw on ReDoS-ish PAT'); + // The literal '(a+)+$' is not present in the action text as a substring → no warn. + // (If new RegExp were used, this test would hang before reaching this assertion.) + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `catastrophic PAT '(a+)+$' not literally in action — must not warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.ok(Array.isArray(result.warnings), 'valid result shape'); + }); + + // Case 23 (mutation-catching): cat producer = file-wide → WARN; sed producer = region-scoped → NO warn + test('case 23a — cat pipe: ! cat app/page.py | grep -Eq PAT + sibling → WARN (file-wide via cat)', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! cat app/page.py | grep -Eq 'await .*refresh'", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `cat-piped grep is file-wide — must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.ok(result.valid !== false, 'valid must remain true even when #968 warns'); + }); + + test('case 23b — sed pipe: ! sed -n "12,40p" app/page.py | grep -Eq PAT + sibling → NO warn (region-scoped)', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! sed -n '12,40p' app/page.py | grep -Eq 'await .*refresh'", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `sed-piped grep is region-scoped — must NOT warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 24: awk region → NO warn + test('case 24 — awk region pipe: ! awk \'/^def make_page/,/^def /\' app/page.py | grep -Eq PAT + sibling → NO warn', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! awk '/^def make_page/,/^def /' app/page.py | grep -Eq 'await .*refresh'", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `awk-piped grep is region-scoped — must NOT warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 25: basename non-over-match — different dirs, same basename → NO warn + test('case 25 — basename non-over-match: different dirs same filename → NO warn', () => { + // Task A bans on apps/web/config.py; Task B lists apps/admin/config.py + // Same basename "config.py" but different dirs → must NOT warn + const content = makeTwoTaskPlan({ + taskAFile: 'apps/web/config.py', + taskAGate: "! grep -Eq 'await .*refresh' apps/web/config.py", + taskBFile: 'apps/admin/config.py', + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `different dirs (apps/web/config.py vs apps/admin/config.py) — same basename but must NOT warn, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 26: extensionless known file (Dockerfile) recognized via knownFiles → WARN + test('case 26 — extensionless known file (Dockerfile) via knownFiles → WARN', () => { + // Task A has ! grep -Eq 'FROM scratch' Dockerfile + // Dockerfile has no extension, so looksLikePath would miss it — but knownFiles should catch it + // Task B lists Dockerfile in and action requires 'FROM scratch' + const content = makeTwoTaskPlan({ + taskAFile: 'Dockerfile', + taskAGate: "! grep -Eq 'FROM scratch' Dockerfile", + taskBFile: 'Dockerfile', + taskBAction: 'Update the image base: FROM scratch ensures minimal surface area.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `Dockerfile (extensionless, known via ) should be recognized — must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.ok(result.valid !== false, 'valid must remain true'); + }); + + // Case 27: wildcard semantic match — patternRequiredIn handles .* correctly + test('case 27 — wildcard semantic match: "await .*refresh" (gate) warns when action has "await bridge.refresh()"', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `patternRequiredIn must match "await .*refresh" against "await bridge.refresh()" — must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 4b: same-file positive control — sibling lists the SAME banned file + requires PAT → WARN + // Paired with case 4: proves the no-warn in case 4 is due to the file mismatch, not a dead detector. + test('case 4b — same-file positive control: sibling lists same file → WARN (proves case 4 no-warn is file-mismatch)', () => { + const content = makeTwoTaskPlan({ + taskAFile: 'app/page.py', + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBFile: 'app/page.py', + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `same-file sibling must warn — proves case 4's no-warn is due to file mismatch, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 7b: non-inverted positive control — without -v the ban IS detected → WARN + // Paired with case 7: proves the -v skip is what suppresses case 7. + test('case 7b — non-inverted positive control: ! grep -q (no -v) + sibling → WARN (proves case 7 no-warn is -v skip)', () => { + const content = makeTwoTaskPlan({ + taskAGate: "! grep -q 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `non-inverted ! grep -q must warn — proves the -v flag is what suppresses case 7, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 25b: basename-fallback positive — bare unqualified filename matches sibling's qualified path → WARN + // Paired with case 25: proves the bare-name basename fallback at src ~line 525 actually fires. + // Case 25 only proves qualified paths don't over-match; this proves the bare fallback does fire. + test('case 25b — basename-fallback positive: bare gate file matches sibling qualified path → WARN (proves basename fallback fires)', () => { + // Task A gate uses bare "config.py" (no directory prefix — unqualified). + // Task B lists "apps/admin/config.py" (qualified). basename("apps/admin/config.py") === "config.py". + // The basename fallback (line 525) should match → WARN. + const content = makeTwoTaskPlan({ + taskAFile: 'config.py', + taskAGate: "! grep -Eq 'await .*refresh' config.py", + taskBFile: 'apps/admin/config.py', + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `bare gate file "config.py" must match sibling "apps/admin/config.py" via basename fallback — must warn, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 28: anchored pattern warns after ^ strip — proves anchor stripping works + // Gate: ! grep -Eq '^FROM scratch' Dockerfile + // Sibling B lists Dockerfile, action requires 'FROM scratch' (no anchor in prose). + // Without anchor stripping, "^FROM scratch" would be treated as containing metacharacters + // and fall back to literal-substring: "^FROM scratch" not in B's prose → no warn. + // With anchor stripping, "FROM scratch" is the effective literal → found in B's prose → WARN. + test('case 28 — anchored pattern warns: ! grep -Eq \'^FROM scratch\' Dockerfile + sibling → WARN (proves ^ strip)', () => { + const content = makeTwoTaskPlan({ + taskAFile: 'Dockerfile', + taskAGate: "! grep -Eq '^FROM scratch' Dockerfile", + taskBFile: 'Dockerfile', + taskBAction: 'Update the image base: FROM scratch ensures minimal surface area.', + }); + const result = scan(content); + assert.ok( + result.warnings.filter(w => w.includes('#968')).length >= 1, + `anchored pattern "^FROM scratch" must warn after ^ is stripped — "FROM scratch" is in sibling action, got: ${JSON.stringify(result.warnings)}`, + ); + assert.strictEqual(result.valid, true, '#968 is warn-only: valid must be true'); + }); + + // Case 29: alternation falls back conservatively — documents the known limitation. + // Gate: ! grep -Eq 'debug|trace' src/logger.ts + // Sibling B lists src/logger.ts, action says "remove debug calls" (contains "debug" but NOT "debug|trace"). + // patternRequiredIn sees unhandled `|` in joined frags → literal-substring fallback on raw pattern. + // "debug|trace" is NOT literally in B's prose → conservative NO warn. + // This is intentional: false-negative is the safe direction for a warn-only advisory. + test('case 29 — alternation conservative fallback: "debug|trace" → NO warn (documents alternation limitation)', () => { + // NOTE: This is intended conservative behavior, not a bug. + // patternRequiredIn falls back to literal-substring for patterns containing `|` (alternation), + // because safely expanding alternation without new RegExp would require a mini-parser. + // The literal "debug|trace" is not present verbatim in the action, so no warn fires. + // A planner who writes `debug|trace` gets no advisory — acceptable, since a false-negative + // is always safer than a false-positive for a warn-only gate. + const content = makeTwoTaskPlan({ + taskAFile: 'src/logger.ts', + taskAGate: "! grep -Eq 'debug|trace' src/logger.ts", + taskBFile: 'src/logger.ts', + taskBAction: 'Remove debug calls from the logger module to reduce noise.', + }); + const result = scan(content); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + `alternation pattern "debug|trace" must conservatively NOT warn — literal "debug|trace" not in action, got: ${JSON.stringify(result.warnings)}`, + ); + }); + + // Case 18: empty content → no crash, no #968 warn + test('case 18 — empty content → no crash', () => { + let result; + assert.doesNotThrow(() => { + result = scan(''); + }); + assert.ok(Array.isArray(result.warnings), 'must return { warnings: [] }'); + assert.strictEqual( + result.warnings.filter(w => w.includes('#968')).length, + 0, + 'empty content must produce no #968 warn', + ); + }); + + // Case 18b: no-task plan → no crash + test('case 18b — no-task plan → no crash', () => { + const content = [ + '---', + 'phase: 01-test', + 'plan: 01', + 'type: execute', + 'wave: 1', + 'depends_on: []', + 'files_modified: []', + 'autonomous: true', + 'must_haves:', + ' - AC1', + '---', + '', + '# No tasks here', + ].join('\n'); + let result; + assert.doesNotThrow(() => { + result = scan(content); + }); + assert.strictEqual(result.warnings.filter(w => w.includes('#968')).length, 0); + }); +}); + +// ─── Group 2: end-to-end via runGsdTools ────────────────────────────────────── + +describe('scanFileWideNegativeGateConflict — end-to-end via verify plan-structure', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + // Case 19: integration — valid stays true despite warning (warn-only) + test('case 19 — integration: valid===true despite #968 warning', () => { + const planContent = makeTwoTaskPlan({ + taskAGate: "! grep -Eq 'await .*refresh' app/page.py", + taskBAction: 'Add a post-reindex handler that calls await bridge.refresh() to repopulate state.', + }); + const planDir = path.join(tmpDir, '.planning', 'phases', '01-test'); + fs.mkdirSync(planDir, { recursive: true }); + fs.writeFileSync(path.join(planDir, '01-01-PLAN.md'), planContent); + + const result = runGsdTools( + 'verify plan-structure .planning/phases/01-test/01-01-PLAN.md', + tmpDir, + ); + const parsed = JSON.parse(result.output); + assert.strictEqual( + parsed.valid, + true, + `#968 is warn-only: valid must be true, got: ${JSON.stringify(parsed)}`, + ); + assert.ok( + parsed.warnings.some(w => w.includes('#968')), + `must have a #968 warning, got: ${JSON.stringify(parsed.warnings)}`, + ); + }); +}); + +// ─── Group 3: doc-contract ──────────────────────────────────────────────────── + +describe('doc-contract: guidance prose is in place', () => { + // Case 20: gsd-planner.md has the new guidance + test('case 20 — gsd-planner.md contains Region-scoped negative gates + reference', () => { + const content = fs.readFileSync(PLANNER_MD, 'utf8'); + assert.ok( + content.includes('Region-scoped negative gates'), + 'gsd-planner.md must include "Region-scoped negative gates"', + ); + assert.ok( + content.includes('planner-antipatterns.md'), + 'gsd-planner.md must reference planner-antipatterns.md', + ); + }); + + // Case 21: planner-antipatterns.md has the new section + test('case 21 — planner-antipatterns.md has ## Region-Scoped Negative Gates + examples', () => { + const content = fs.readFileSync(ANTIPATTERNS_MD, 'utf8'); + assert.ok( + content.includes('## Region-Scoped Negative Gates'), + 'planner-antipatterns.md must include "## Region-Scoped Negative Gates"', + ); + assert.ok( + content.includes('await .*refresh'), + 'planner-antipatterns.md must include the worked example pattern "await .*refresh"', + ); + // Verify sed or awk region example is present + const hasSedOrAwk = content.includes('sed -n') || content.includes('awk '); + assert.ok( + hasSedOrAwk, + 'planner-antipatterns.md must include sed-n or awk region example', + ); + }); +}); + +// ─── Group 4: AC3 executable proof ─────────────────────────────────────────── + +describe('AC3: executable proof — file-wide ban vs region-scoped simultaneously satisfiable', () => { + test('case 22 — grep/sed proof: both gates simultaneously satisfiable', () => { + // Check if grep and sed are available + const grepAvail = spawnSync('grep', ['--version']).status === 0; + const sedAvail = spawnSync('sed', ['--version']).status === 0 || + spawnSync('sed', ['-n', '1p', '/dev/null']).status === 0; + + if (!grepAvail || !sedAvail) { + // Skip gracefully if tools are unavailable + return; + } + + // Write a temp Python file with: + // def make_page(): — no await refresh + // async def reindex_handler(): — awaits bridge.refresh() + const tmpFile = path.join(os.tmpdir(), `gsd-968-proof-${process.pid}.py`); + const pyContent = [ + 'def make_page():', + ' """Synchronous factory — must not block on a refresh."""', + ' return {"title": "My Page"}', + '', + '', + 'async def reindex_handler():', + ' """Post-reindex callback — must await bridge.refresh() to repopulate state."""', + ' await bridge.refresh()', + ' return True', + ].join('\n'); + fs.writeFileSync(tmpFile, pyContent); + + try { + // (a) File-wide: grep -Eq 'await .*refresh' — should EXIT 0 (pattern found) + // This means a file-wide ban (! grep -Eq ...) WOULD FAIL + const fileWide = spawnSync('grep', ['-Eq', 'await .*refresh', tmpFile]); + assert.strictEqual( + fileWide.status, + 0, + 'grep file-wide should find the pattern (exits 0) — proving the file-wide ban would fail', + ); + + // (b) Region-scoped (make_page only): sed extracts lines 1-3, piped to grep → pattern NOT found + // The factory region is clean: ban PASSES + const makePageLines = spawnSync('sed', ['-n', '1,3p', tmpFile]); + assert.strictEqual(makePageLines.status, 0, 'sed should succeed'); + const makePageRegion = makePageLines.stdout.toString(); + + // Write to a temp file and grep it + const regionFile = path.join(os.tmpdir(), `gsd-968-region-${process.pid}.py`); + fs.writeFileSync(regionFile, makePageRegion); + try { + const regionBan = spawnSync('grep', ['-Eq', 'await .*refresh', regionFile]); + assert.strictEqual( + regionBan.status, + 1, + 'grep in make_page region should NOT find pattern (exits 1) — ban PASSES in factory region', + ); + + // (c) Region-scoped (reindex_handler): grep should FIND the pattern → requirement met + const reindexLines = spawnSync('sed', ['-n', '6,9p', tmpFile]); + const reindexRegion = reindexLines.stdout.toString(); + const reindexFile = path.join(os.tmpdir(), `gsd-968-reindex-${process.pid}.py`); + fs.writeFileSync(reindexFile, reindexRegion); + try { + const reindexCheck = spawnSync('grep', ['-Eq', 'await .*refresh', reindexFile]); + assert.strictEqual( + reindexCheck.status, + 0, + 'grep in reindex_handler region MUST find pattern (exits 0) — requirement met', + ); + } finally { + try { fs.unlinkSync(reindexFile); } catch { /* ignore */ } + } + } finally { + try { fs.unlinkSync(regionFile); } catch { /* ignore */ } + } + } finally { + try { fs.unlinkSync(tmpFile); } catch { /* ignore */ } + } + }); +}); diff --git a/tests/init.test.cjs b/tests/init.test.cjs index cb554c2ab..e9456d36a 100644 --- a/tests/init.test.cjs +++ b/tests/init.test.cjs @@ -325,6 +325,142 @@ describe('init commands', () => { const output = JSON.parse(result.output); assert.strictEqual(output.phase_req_ids, null); }); + + test('init plan-phase resolves phase_req_ids from flat Phase Details after active milestone heading', () => { + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '11-second-active-phase'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), [ + '---', + 'milestone: v0.4.0', + 'current_phase: 11', + '---', + '', + ].join('\n')); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), [ + '# Roadmap: Example', + '', + '## Milestones', + '', + '- ✅ **v0.3.0 Foundations** - Phases 1-9 (shipped 2026-01-01)', + '- 🚧 **v0.4.0 Feature Work** - Phases 10-11 (in progress)', + '', + '## Phases', + '', + '
', + '✅ v0.3.0 Foundations (Phases 1-9) - SHIPPED 2026-01-01', + '', + '- [x] **Phase 1: Bootstrap**', + '', + '
', + '', + '### 🚧 v0.4.0 Feature Work (Active)', + '', + '**Milestone Goal:** Deliver the feature set.', + '', + '- [ ] **Phase 10: First Active Phase**', + '- [ ] **Phase 11: Second Active Phase**', + '', + '### 📋 v0.5+ (Planned)', + '', + '## Phase Details', + '', + '### Phase 10: First Active Phase', + '**Goal**: Build the first piece.', + '**Requirements**: REQ-01', + '', + '### Phase 11: Second Active Phase', + '**Goal**: Build the second piece.', + '**Requirements**: REQ-02, REQ-03', + '', + '## Progress', + '', + ].join('\n')); + + const result = runGsdTools('init plan-phase 11', tmpDir); + assert.ok(result.success, `init plan-phase failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_found, true); + assert.strictEqual(output.phase_req_ids, 'REQ-02, REQ-03'); + }); + + test('init execute-phase resolves phase_req_ids from flat Phase Details after active milestone heading', () => { + seedPhase(tmpDir, '11-second-active-phase', { + '11-01-PLAN.md': '# Plan', + }); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), [ + '---', + 'milestone: v0.4.0', + 'current_phase: 11', + '---', + '', + ].join('\n')); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), [ + '# Roadmap: Example', + '', + '## Phases', + '', + '### 🚧 v0.4.0 Feature Work (Active)', + '', + '- [ ] **Phase 10: First Active Phase**', + '- [ ] **Phase 11: Second Active Phase**', + '', + '### 📋 v0.5+ (Planned)', + '', + '## Phase Details', + '', + '### Phase 10: First Active Phase', + '**Goal**: Build the first piece.', + '**Requirements**: REQ-01', + '', + '### Phase 11: Second Active Phase', + '**Goal**: Build the second piece.', + '**Requirements**: REQ-02, REQ-03', + '', + ].join('\n')); + + const result = runGsdTools('init execute-phase 11', tmpDir); + assert.ok(result.success, `init execute-phase failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_found, true); + assert.strictEqual(output.phase_req_ids, 'REQ-02, REQ-03'); + }); + + test('init phase-op resolves a details-summary milestone phase from later flat Phase Details', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), [ + 'milestone: v1.11', + 'current_phase: 86', + '', + ].join('\n')); + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), [ + '# Roadmap', + '', + '## Phases', + '
', + '🔄 v1.11 A06 (Phases 86-91) — IN PROGRESS', + '', + '- [ ] **Phase 86: Details Block Regression** — Parser should resolve this (DATA-01)', + '- [ ] **Phase 87: Other Work** — Later phase', + '
', + '', + '## Phase Details', + '', + '### Phase 86: Details Block Regression', + '**Goal**: Resolve phase details after collapsed milestone block', + '**Requirements**: DATA-01', + '', + '### Phase 87: Other Work', + '**Goal**: Not relevant', + '', + ].join('\n')); + + const result = runGsdTools('init phase-op 86', tmpDir); + assert.ok(result.success, `init phase-op failed: ${result.error}`); + + const output = JSON.parse(result.output); + assert.strictEqual(output.phase_found, true); + assert.strictEqual(output.phase_name, 'Details Block Regression'); + }); }); // ───────────────────────────────────────────────────────────────────────────── diff --git a/tests/issue-766-plugin-manifest.test.cjs b/tests/issue-766-plugin-manifest.test.cjs index 34cdb8a00..fbf627428 100644 --- a/tests/issue-766-plugin-manifest.test.cjs +++ b/tests/issue-766-plugin-manifest.test.cjs @@ -24,6 +24,7 @@ const ROOT = path.resolve(__dirname, '..'); const identity = require(path.join(ROOT, 'gsd-core', 'bin', 'lib', 'package-identity.cjs')); const pkg = require(path.join(ROOT, 'package.json')); const { MANAGED_HOOKS } = require(path.join(ROOT, 'hooks', 'managed-hooks-registry.cjs')); +const { cleanup } = require('./helpers.cjs'); const PLUGIN_JSON_PATH = path.join(ROOT, '.claude-plugin', 'plugin.json'); const HOOKS_JSON_PATH = path.join(ROOT, 'hooks', 'hooks.json'); @@ -235,9 +236,9 @@ describe('B: hooks/hooks.json', () => { // or changes `name` to an invalid form goes red immediately. // // C2 (OPPORTUNISTIC) — When the `claude` binary IS on PATH, also run -// `claude plugin validate . --strict` as an end-to-end smoke test. -// This tier provides defence-in-depth for schema changes Claude Code -// may introduce that the fixture hasn't yet captured. +// `claude plugin validate --strict` as an end-to-end +// smoke test. This tier provides defence-in-depth for schema changes +// Claude Code may introduce that the fixture hasn't yet captured. // describe('C: plugin.json schema validation', () => { @@ -366,19 +367,29 @@ describe('C: plugin.json schema validation', () => { })(); test( - 'C2: claude plugin validate . --strict exits 0 (opportunistic — skip when claude not on PATH)', + 'C2: claude plugin validate --strict exits 0 (opportunistic — skip when claude not on PATH)', { skip: !claudeAvailable ? 'claude binary not on PATH' : false }, () => { - const result = spawnSync('claude', ['plugin', 'validate', '.', '--strict'], { - cwd: ROOT, - encoding: 'utf-8', - timeout: 15000, - }); - assert.equal( - result.status, - 0, - `claude plugin validate . --strict exited with ${result.status}.\nstdout: ${result.stdout}\nstderr: ${result.stderr}` - ); + const pluginRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-plugin-validate-')); + try { + fs.mkdirSync(path.join(pluginRoot, '.claude-plugin'), { recursive: true }); + fs.copyFileSync(PLUGIN_JSON_PATH, path.join(pluginRoot, '.claude-plugin', 'plugin.json')); + fs.symlinkSync(path.join(ROOT, 'commands'), path.join(pluginRoot, 'commands'), 'dir'); + fs.symlinkSync(path.join(ROOT, 'hooks'), path.join(pluginRoot, 'hooks'), 'dir'); + + const result = spawnSync('claude', ['plugin', 'validate', pluginRoot, '--strict'], { + cwd: ROOT, + encoding: 'utf-8', + timeout: 15000, + }); + assert.equal( + result.status, + 0, + `claude plugin validate ${pluginRoot} --strict exited with ${result.status}.\nstdout: ${result.stdout}\nstderr: ${result.stderr}` + ); + } finally { + cleanup(pluginRoot); + } } ); }); diff --git a/vitest.config.ts b/vitest.config.ts deleted file mode 100644 index b0200d123..000000000 --- a/vitest.config.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { defineConfig } from 'vitest/config'; - -export default defineConfig({ - test: { - projects: [ - { - test: { - name: 'unit', - root: './sdk', - include: ['src/**/*.test.ts'], - exclude: ['src/**/*.integration.test.ts'], - }, - }, - { - test: { - name: 'integration', - root: './sdk', - include: ['src/**/*.integration.test.ts'], - testTimeout: 120_000, - }, - }, - ], - }, -});