diff --git a/.changeset/proud-birds-travel.md b/.changeset/proud-birds-travel.md new file mode 100644 index 000000000..61e009949 --- /dev/null +++ b/.changeset/proud-birds-travel.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 3261 +--- +**Complexity-triggered refactor proposals** — after a phase runs, GSD can now measure the complexity of the code that phase touched and surface a scoped refactor proposal when a function crosses a threshold or drifts past its recorded anchor, so entropy gets caught while it is still one function instead of a rewrite. Advisory and off by default; enable with `gsd config-set refactor.trigger_enabled true`. (#1953) diff --git a/.gitignore b/.gitignore index 5918e6829..1d38502b9 100644 --- a/.gitignore +++ b/.gitignore @@ -69,6 +69,8 @@ build/ /tsconfig.build.tsbuildinfo /gsd-core/bin/lib/commonjs-marker.cjs /gsd-core/bin/lib/broken-windows.cjs +/gsd-core/bin/lib/complexity-trigger.cjs +/gsd-core/bin/lib/refactor-trigger-command-router.cjs /gsd-core/bin/lib/host-integration.cjs /gsd-core/bin/lib/host-integration-sdk.cjs /gsd-core/bin/lib/host-integration-adapters/imperative-hook-bus.cjs diff --git a/CONTEXT.md b/CONTEXT.md index a2609b062..5de265298 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -145,7 +145,10 @@ Workflow contract seam covering agent worktree lifecycle orchestration rules. Th Adapter Module owning linked-worktree root mapping and metadata-prune policy (`git worktree prune` non-destructive default) for planning/workstream callers. ### Git Query Module -Module owning bounded, never-throw git repository introspection — the single seam for read-only git queries that degrade gracefully rather than throwing. **Adapter 1 — base-branch detection** (`gsd_run query git.base-branch`): Implements a full precedence ladder: (1) `git.base_branch` config override from `.planning/config.json`; (2) `git symbolic-ref --short refs/remotes/origin/HEAD`; (3) `git remote show origin` HEAD branch (authoritative when origin/HEAD is unset — the common case for `git init + remote add + fetch` without `set-head`); (4) local branch existence (`master` present and `main` absent → `master`; `main` present → `main`); (5) `"main"` last-resort default. All git subprocesses are bounded with timeouts (5–15 s) and degrade gracefully to the next tier; the function never throws. Replaces duplicated per-workflow bash detection that silently fell through to `:-main` on master repos (#1146). **Adapter 2 — worktree-info detection**: `gitWorktreeInfoInternal` (`git rev-parse --is-inside-work-tree` + `--show-toplevel`), absorbed from the Core module when the `core.cjs` re-export spine was retired and aligned to this module's bounded-timeout / degrade-don't-throw convention (worktree-info detection is a query concern, distinct from the Worktree Safety Policy Module's lifecycle policy). Source: `src/git-base-branch.cts` → `gsd-core/bin/lib/git-base-branch.cjs`. Wired into `execute-phase.md`, `quick.md`, `ship.md`, `complete-milestone.md`, and `pr-branch.md`. +Module owning bounded, never-throw git repository introspection — the single seam for read-only git queries that degrade gracefully rather than throwing. **Adapter 1 — base-branch detection** (`gsd_run query git.base-branch`): Implements a full precedence ladder: (1) `git.base_branch` config override from `.planning/config.json`; (2) `git symbolic-ref --short refs/remotes/origin/HEAD`; (3) `git remote show origin` HEAD branch (authoritative when origin/HEAD is unset — the common case for `git init + remote add + fetch` without `set-head`); (4) local branch existence (`master` present and `main` absent → `master`; `main` present → `main`); (5) `"main"` last-resort default. All git subprocesses are bounded with timeouts (5–15 s) and degrade gracefully to the next tier; the function never throws. Replaces duplicated per-workflow bash detection that silently fell through to `:-main` on master repos (#1146). **Adapter 2 — worktree-info detection**: `gitWorktreeInfoInternal` (`git rev-parse --is-inside-work-tree` + `--show-toplevel`), absorbed from the Core module when the `core.cjs` re-export spine was retired and aligned to this module's bounded-timeout / degrade-don't-throw convention (worktree-info detection is a query concern, distinct from the Worktree Safety Policy Module's lifecycle policy). **Adapter 3 — phase change-set detection** (#1953): `phaseStartCommit` resolves the commit that ADDED a phase's `PLAN.md` (`git log --diff-filter=A -1`) — the anchor for "what did this phase touch", since STATE.md records no phase-start sha — and `changedFilesSince` returns the changed paths from that anchor to HEAD. `changedFilesSince` uses `-z` with `core.quotepath=false` and splits on NUL, because git otherwise quotes and escapes non-ASCII paths (lossy round-trip) and a `\n` split corrupts a filename containing a newline; the ref precedes a literal `--` so a dash-leading path cannot be read as an option. Both bounded at 15 s and both degrade to `null`. Consumed by the Complexity Trigger Module. Source: `src/git-base-branch.cts` → `gsd-core/bin/lib/git-base-branch.cjs`. Wired into `execute-phase.md`, `quick.md`, `ship.md`, `complete-milestone.md`, and `pr-branch.md`. + +### Complexity Trigger Module +Leaf module (imports only `node:fs`/`node:path`) owning per-function complexity measurement and the refactor-proposal decision, behind the opt-in `refactor-trigger` capability (#1953, ADR-1953). `analyzeSource` scores each function by **decision-point counting** over comment- and literal-stripped source — base 1 plus one per `if`/`else if`/`for`/`while`/`do`/`case`/`catch`/`&&`/`||`/`?:`, explicitly NOT `?.`, `??`, bare `else`, or `default:`. The stripper preserves length and newlines so line numbers survive, keeps `${…}` interpolations as code, and disambiguates a regex literal from division by the preceding significant token; an unterminated literal returns `REFACTOR_ANALYZER_UNPARSEABLE` rather than an approximate number, because a silently-wrong score is worse than no score. `evaluateCandidates` flags a function when its score exceeds `refactor.complexity_threshold` **or** its growth over its anchor exceeds `refactor.complexity_jump_delta` — both strictly greater, matching ESLint's `complexity: {max: N}`. The baseline is an **anchor**, not a rolling value: set on first observation, never advanced by a plain evaluate, moved only by `reanchorBaseline` on disposition — so the delta accumulates since the last conscious decision and slow creep is caught a phase before the absolute threshold reaches it. Stored at `.planning/complexity-baseline.json`; proposals at `${PHASE_DIR}/${NN}-REFACTOR.md`. Frozen `REASON`/`VERDICT` enums are the typed surface tests assert against. _Avoid_: "the complexity gate" — this capability declares no gate; strict mode records a `deviation` window and the Broken-Windows Ledger's ship gate does the blocking. Sources: `src/complexity-trigger.cts`, `src/refactor-trigger-command-router.cts` (CLI family `gsd-tools refactor`). ### Runtime Name Policy Module Module owning runtime identity normalization at runtime-selection seams. Canonicalizes alias signals from env/config (`GSD_RUNTIME`, `.planning/config.json:runtime`) to supported runtime IDs so output emitters and query runtime gates stay consistent across naming variants (for example `codex-app`/`codex-cli` -> `codex`). Sources: `gsd-core/bin/lib/runtime-name-policy.cjs`, alias manifest `gsd-core/bin/shared/runtime-aliases.manifest.json`. diff --git a/capabilities/refactor-trigger/capability.json b/capabilities/refactor-trigger/capability.json new file mode 100644 index 000000000..00faf7e36 --- /dev/null +++ b/capabilities/refactor-trigger/capability.json @@ -0,0 +1,67 @@ +{ + "id": "refactor-trigger", + "role": "feature", + "version": "1.10.0", + "title": "Complexity-triggered refactor", + "description": "Measures the complexity of the code a phase touched and, when a function crosses a configured threshold or jumps past its recorded anchor, surfaces a scoped refactor proposal at .planning/phases//-REFACTOR.md. Advisory by default — it never edits code and never blocks. Opt-in strict mode blocks /gsd-ship while a proposal is untriaged; a declined proposal is recorded in the broken-windows ledger when that capability is present. Operationalizes 'refactor early, refactor often' as continuous pressure instead of a thing you have to remember (issue #1953).", + "tier": "full", + "requires": [], + "engines": { + "gsd": ">=1.10.0" + }, + "runtimeCompat": { + "supported": [ + "*" + ], + "unsupported": [] + }, + "skills": [], + "agents": [], + "activationKey": "refactor.trigger_enabled", + "config": { + "refactor.trigger_enabled": { + "type": "boolean", + "default": false, + "description": "Enable the complexity-triggered refactor hook. When true, an execute:post step evaluates the complexity of the files the phase touched and writes a scoped refactor proposal if a function crosses refactor.complexity_threshold or jumps past refactor.complexity_jump_delta. Opt-in; when false the hook never runs. Issue #1953." + }, + "refactor.complexity_threshold": { + "type": "number", + "default": 15, + "description": "Absolute per-function complexity above which a refactor proposal is surfaced. Semantics match ESLint's `complexity: {max: N}` — the trigger is STRICTLY GREATER, so a score of exactly N does not trigger. Default 15 follows SonarSource's default; ESLint's own default is 20 and radon's rank C begins at 11. Raise it if proposals feel like noise." + }, + "refactor.complexity_jump_delta": { + "type": "number", + "default": 5, + "description": "Complexity growth above which a refactor proposal is surfaced even when the absolute threshold is not reached. Measured against the function's anchor — the score recorded the last time the function was consciously dispositioned — so it accumulates across phases and catches slow creep the absolute threshold would miss. Strictly greater, as with the threshold." + }, + "refactor.trigger_strict": { + "type": "boolean", + "default": false, + "description": "Record an untriaged refactor proposal as an open `deviation` entry in the broken-windows ledger, so it becomes a tracked task that must be resolved before ship. Off by default and deliberately so: a blocking complexity number is a metric an executor can satisfy by splitting one coherent function into two incoherent ones, so the entry clears on the proposal being DISPOSITIONED (gsd-tools refactor accept|decline), never on the score improving. Ship blocking is the broken-windows capability's existing ship:pre gate — enable it with workflow.windows_enforce. With broken-windows absent, strict mode still records the proposal locally and says so; it cannot block on its own. Advisory mode (the default) surfaces the same proposal and tracks nothing." + } + }, + "commands": [ + { + "family": "refactor", + "module": "refactor-trigger-command-router.cjs", + "router": "routeRefactorTriggerCommand" + } + ], + "hooks": [], + "steps": [ + { + "point": "execute:post", + "ref": { + "command": "refactor evaluate" + }, + "produces": [ + "REFACTOR.md" + ], + "consumes": [], + "when": "refactor.trigger_enabled", + "onError": "skip" + } + ], + "contributions": [], + "gates": [] +} diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index 95a4fe9bb..cc8e440eb 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -1341,6 +1341,32 @@ The `API-SURFACE.md` output lists exported symbols (functions, classes, decorato --- +### `gsd-tools refactor` + +Evaluate the complexity of the files a phase touched and surface a scoped refactor proposal when a function's score crosses `refactor.complexity_threshold` or jumps past its recorded anchor by more than `refactor.complexity_jump_delta`. Gated on `refactor.trigger_enabled: true` in `config.json` (see [Configuration Reference](CONFIGURATION.md#refactor-trigger-settings)); when disabled, every subcommand prints an activation hint and stops — it is inert otherwise. + +| Subcommand | Description | +|------------|-------------| +| `evaluate --phase [--since ] [--raw]` | Analyze files changed since the phase's start commit (or `--since `) and write a `-REFACTOR.md` proposal when a candidate triggers | +| `status [--phase ] [--raw]` | List all recorded proposals across phases, or show the proposal for one phase | +| `accept --phase [--raw]` | Disposition the phase's untriaged proposal as accepted; re-anchors the target function's baseline to its current score | +| `decline --phase --reason "" [--raw]` | Disposition the phase's untriaged proposal as declined with a recorded reason; re-anchors the baseline the same way | + +**Produces:** `.planning/phases//-REFACTOR.md` (from `evaluate`, only when a candidate triggers) + +```bash +node gsd-tools.cjs refactor evaluate --phase 3 # Evaluate phase 3's touched files +node gsd-tools.cjs refactor evaluate --phase 3 --since abc123 # Evaluate against a specific ref +node gsd-tools.cjs refactor status # List all recorded proposals +node gsd-tools.cjs refactor status --phase 3 # Show phase 3's proposal +node gsd-tools.cjs refactor accept --phase 3 # Accept phase 3's proposal +node gsd-tools.cjs refactor decline --phase 3 --reason "flat dispatch table, not a real hotspot" # Decline with a reason +``` + +Trigger semantics match ESLint's `complexity: {max: N}` — strictly greater, so a score exactly equal to `refactor.complexity_threshold` does not trigger. The jump check compares against the function's anchor (the score recorded the last time it was accepted or declined), not the single-phase change, so it accumulates across phases until dispositioned. `refactor accept`/`refactor decline` are the only actions that clear a tracked proposal — the score improving on its own does not. See [ADR-1953](adr/1953-complexity-triggered-refactor.md). + +--- + ## AI Integration Commands ### `/gsd-ai-integration-phase` diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index e9b37fd3c..fd296b60a 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -778,6 +778,18 @@ A CI-built graph rebuilt minutes ago against an old checkout will read as fresh on mtime but `commit_stale: true`. Surface both when answering architecture questions. + +### Refactor-Trigger Settings + +| Setting | Type | Default | Description | +|---------|------|---------|-------------| +| `refactor.trigger_enabled` | boolean | `false` | Enable the complexity-triggered refactor hook. When `true`, an `execute:post` step evaluates the complexity of the files the phase touched and writes a scoped refactor proposal if a function crosses `refactor.complexity_threshold` or jumps past `refactor.complexity_jump_delta`. Opt-in; when `false` the hook never runs. Added in v1.10.0 (#1953) | +| `refactor.complexity_threshold` | number | `15` | Absolute per-function complexity above which a refactor proposal is surfaced. Semantics match ESLint's `complexity: {max: N}` — the trigger is strictly greater, so a score of exactly `N` does not trigger. Default `15` follows SonarSource's default; ESLint's own default is `20` and radon's rank C begins at `11`. Raise it if proposals feel like noise. Added in v1.10.0 | +| `refactor.complexity_jump_delta` | number | `5` | Complexity growth above which a refactor proposal is surfaced even when the absolute threshold is not reached. Measured against the function's anchor — the score recorded the last time the function was consciously dispositioned (`refactor accept`/`refactor decline`) — so it accumulates across phases and catches slow creep the absolute threshold would miss. Strictly greater, as with the threshold. Added in v1.10.0 | +| `refactor.trigger_strict` | boolean | `false` | Record an untriaged refactor proposal as an open `deviation` entry in the broken-windows ledger, so it becomes a tracked task that must be resolved before ship. Off by default and deliberately so: a blocking complexity number is a metric an executor can satisfy by splitting one coherent function into two incoherent ones, so the entry clears on the proposal being dispositioned (`gsd-tools refactor accept\|decline`), never on the score improving. Ship blocking is the broken-windows capability's existing `ship:pre` gate — enable it separately with `workflow.windows_enforce`. With broken-windows absent, strict mode still records the proposal locally and says so; it cannot block on its own. Enabling `refactor.trigger_strict` without also enabling `workflow.windows_enforce` (or with broken-windows not installed) surfaces a typed `refactor_strict_not_enforcing` warning on every triggering `refactor evaluate`, naming the exact remediation. Advisory mode (the default) surfaces the same proposal and tracks nothing. Added in v1.10.0 | + +See [ADR-1953](adr/1953-complexity-triggered-refactor.md) for the design rationale, including why the anchor moves only on disposition and never on the score improving. + ### Usage ```bash diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 23ae9309e..6704783ca 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -3371,3 +3371,13 @@ The load-bearing wire is the `plan-phase` lift into `must_haves.prohibitions`, s **Backward compatibility:** A project with no `.planning/WINDOWS.md` reports `open_count: 0` and ships cleanly; the gate only activates once windows are recorded. **Configuration:** `graphify.graph_path` + +### 159. Complexity-Triggered Refactor + +**Behavior:** An `execute:post` step measures the complexity of the files a phase touched (decision-point counting over comment- and literal-stripped source, no external dependency) and surfaces a scoped refactor proposal at `.planning/phases//-REFACTOR.md` when a function's score exceeds `refactor.complexity_threshold` or its growth over its recorded anchor exceeds `refactor.complexity_jump_delta` — whichever trips first, both reported. Trigger semantics are strictly greater (ESLint's `complexity: {max: N}` convention), so a score exactly equal to the threshold does not trigger. The anchor is set the first time a function is observed and moves only when the proposal is dispositioned via `refactor accept` or `refactor decline` — never when the score alone improves — so the jump delta is cumulative growth since the last conscious decision about that function, not the change made in a single phase. Advisory by default: the proposal is informational only, never edits code, and never blocks. Opt-in `refactor.trigger_strict` records an untriaged proposal as an open `deviation` entry in the broken-windows ledger (#1950) instead — it does not block on its own; ship-blocking is broken-windows' existing `ship:pre` gate, enabled separately with `workflow.windows_enforce`. Without broken-windows installed, strict mode still records the proposal locally and says so. Enabling `refactor.trigger_strict` without `workflow.windows_enforce` also on (or with broken-windows absent) surfaces a typed `refactor_strict_not_enforcing` warning on every triggering evaluate, naming the exact remediation, so this enforcement gap is never silent. A declined proposal resolves its ledger entry as `waived` with the recorded reason; an accepted one resolves as `fixed`. The metric is approximate by construction: biased against a flat `switch`, blind to nesting depth, JS/TS-family only, and a renamed function loses its anchor (issue #1953). + +**Commands:** `gsd-tools refactor evaluate | status | accept | decline`. + +**Config:** `refactor.trigger_enabled` (master gate, default `false`), `refactor.complexity_threshold` (default `15`), `refactor.complexity_jump_delta` (default `5`), `refactor.trigger_strict` (default `false`). See [Configuration Reference](CONFIGURATION.md#refactor-trigger-settings). + +**Backward compatibility:** Off by default. When `refactor.trigger_enabled` is `false` the hook never runs and writes nothing; a project that never enables it is completely unaffected. diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index f01eceb31..3df383224 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -345,6 +345,7 @@ "command-routing-hub.cjs", "commands.cjs", "commonjs-marker.cjs", + "complexity-trigger.cjs", "config-loader.cjs", "config-schema.cjs", "config-types.cjs", @@ -425,6 +426,7 @@ "prohibition-enforcement.cjs", "project-root.cjs", "prompt-budget.cjs", + "refactor-trigger-command-router.cjs", "research-provider.cjs", "research-store.cjs", "resolution.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index e51d56e59..131afa2e3 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -530,6 +530,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `profile-pipeline-command-router.cjs` | ADR-959 capability command router for the profile-pipeline command family — dispatches scan-sessions, extract-messages, profile-sample (pipeline phase) and write-profile, profile-questionnaire, generate-dev-preferences, generate-claude-profile, generate-claude-md (output phase); phase 6 cutover | | `profile-pipeline.cjs` | User behavioral profiling data pipeline, session file scanning | | `prompt-budget.cjs` | Pure token-budget accounting for review prompts — estimates tokens, applies deterministic trim priority (head-shrink PROJECT.md, proportional plan truncation, drop context/research/requirements, hard-fail guard), returns structured metadata for `review.max_prompt_tokens` (#3081) | +| `refactor-trigger-command-router.cjs` | ADR-959 capability command router for `gsd-tools refactor` (issue #1953) — dispatches evaluate/status/accept/decline subcommands for the complexity-triggered refactor capability; owns capability-activation gating, git invocation (via the `git-base-branch.cjs` `phaseStartCommit`/`changedFilesSince` adapters), config reads, phase-directory resolution, and the optional broken-windows ledger integration around the pure `complexity-trigger.cjs` leaf | | `research-provider.cjs` | Research provider waterfall, confidence tiers, and planResearch (cache-hits + fetch plan) | | `research-store.cjs` | Content-addressed research cache: sha256 keys, per-source TTL staleness, two-tier (user ~/.gsd / project .planning) store | | `retired-artifact-cleanup.cjs` | Manifest-safe cleanup for descriptor-declared retired runtime artifact surfaces; shared by install and profile/surface apply so removed layout kinds converge without deleting modified or unknown user files (#2644) | diff --git a/docs/README.md b/docs/README.md index b7dfaba5c..be16b91c6 100644 --- a/docs/README.md +++ b/docs/README.md @@ -27,6 +27,7 @@ Language versions: [English](README.md) · [Português (pt-BR)](pt-BR/README.md) - [Plan a phase](how-to/plan-a-phase.md) — run research, decompose work, and verify plan quality - [Execute a phase](how-to/execute-a-phase.md) — run plans in parallel waves with fresh-context subagents - [Verify and ship](how-to/verify-and-ship.md) — walk through completed work, diagnose failures, and create the PR +- [Catch complexity before it compounds](how-to/act-on-a-refactor-proposal.md) — enable the post-execute refactor hook, read a proposal's score vs. anchor delta, and accept or decline it - [Run phases autonomously](how-to/run-phases-autonomously.md) — use autonomous mode for unattended phase execution - [Handle quick and fast tasks](how-to/handle-quick-and-fast-tasks.md) — use `/gsd-quick` and `/gsd-fast` for ad-hoc work outside the phase loop - [Configure model profiles](how-to/configure-model-profiles.md) — switch between quality, balanced, and budget model tiers diff --git a/docs/adr/1953-complexity-triggered-refactor.md b/docs/adr/1953-complexity-triggered-refactor.md new file mode 100644 index 000000000..cc3f3372d --- /dev/null +++ b/docs/adr/1953-complexity-triggered-refactor.md @@ -0,0 +1,151 @@ +# ADR-1953: Complexity-triggered refactor — the loop measures the entropy it just added + +- **Status:** Proposed +- **Date:** 2026-08-09 +- **Issue:** [#1953](https://github.com/open-gsd/gsd-core/issues/1953) +- **Implementation:** `src/complexity-trigger.cts`, `src/refactor-trigger-command-router.cts`, `capabilities/refactor-trigger/capability.json`, and a changed-files adapter in `src/git-base-branch.cts` +- **Extends:** [ADR-857](857-capability-system.md) (registers on the `execute:post` extension point) · [ADR-894](894-capability-declaration-format.md) (a `role: "feature"` manifest with `commands`, `steps`, and a `gates` entry) +- **Related:** [#1950](https://github.com/open-gsd/gsd-core/issues/1950) / the `broken-windows` capability — this ADR reuses its ledger rather than adding a second one + +## Context + +*The Pragmatic Programmer* Topic 40 — "Refactor Early, Refactor Often" — argues for +refactoring as gardening: a little, continuously, because entropy compounds. GSD today has +refactoring only as a **manual commit type** (`agents/gsd-executor.md`, `agents/gsd-planner.md`): +an optional cleanup the executor may perform. Nothing measures accumulated complexity and +nothing triggers a refactor. It happens if, and only if, someone remembers. + +That gap is worse under an AI executor than under a human one, and for a structural reason: +**every phase runs in a fresh context**, so no single run ever sees the accumulated mess. +Each phase adds a branch here and a special case there, every one locally justified by "make +the tests pass". By the time a human notices, the hotspot is a rewrite. + +The signal needed to close this is cheap and already sitting there: the loop knows exactly +which files a phase touched, and it knows the moment the phase ends. What is missing is a +trigger. + +## Decision + +**D1 — A capability on `execute:post`, not a change to the loop.** `refactor-trigger` is a +`role: "feature"` capability, `activationKey: refactor.trigger_enabled`, default **off**. +Nothing about the loop changes for anyone who does not opt in. + +**D2 — The signal is computed in-core, with no new dependency.** A decision-point counter +over comment- and literal-stripped source, Node builtins only, in the same regex idiom +`src/intel.cts` already uses for export extraction. This was chosen over the issue's original +"use Memtrace's complexity signals" and over shelling out to ESLint `complexity` / radon. + +The reason is not that the alternatives are worse metrics — they are better ones. It is that +the `execute:post` hook fires as a **deterministic CLI**, not as an agent holding MCP tools, +and core forbids external dependencies. An agent-side Memtrace variant could not be bound by +a behavioral test, which is a hard requirement of the linked issue's own acceptance criteria. +The metric sits behind a named seam so a second one is additive later. + +**D3 — Two numbers, never one composite.** A function is a candidate when its absolute score +exceeds `refactor.complexity_threshold`, **or** when its growth over its anchor exceeds +`refactor.complexity_jump_delta`. Both are reported. Trigger semantics are ESLint's +`complexity: {max: N}` semantics — strictly greater — so a score equal to the threshold does +not trigger. + +**D4 — The baseline is a stable anchor, not a rolling value.** It is set the first time a +function is observed and moves only when the proposal is *dispositioned*. The delta is +therefore cumulative since the last conscious decision about that function. + +A rolling baseline was tried first and discarded: with it, the delta is always the +single-phase change, so a function creeping `+2` per phase against a delta of 5 never trips +the jump check, and the absolute threshold catches the creep first — leaving the jump-delta +contributing nothing. The stable anchor catches that creep a full phase earlier, which is the +entire reason the second number exists. The cost is that a legitimately-growing function +re-proposes until dispositioned. + +**D5 — Advisory by default; strict mode tracks on *disposition*, never on the score.** This is +the load-bearing decision of the whole design. A blocking complexity number is a metric that +an executor optimizing for green gates can satisfy by splitting one coherent function into two +incoherent ones — identical total complexity, worse cohesion. So the tracked entry clears when +the proposal is **dispositioned** — `refactor accept` or `refactor decline`, either one — and +never asks whether the number went down. + +**D6 — This capability declares no gates. Strict mode routes through the broken-windows +ledger.** An untriaged proposal becomes an open `deviation` entry via `appendWindow`; +blocking is broken-windows' existing, already-dispatched `ship:pre` gate, enabled separately +with `workflow.windows_enforce`. A declined proposal resolves its entry as `waived` with the +recorded reason; an accepted one resolves it as `fixed`. There is no second ledger and no +second gate. + +Two earlier cuts of this decision were wrong and are recorded because the reason generalizes. +A `check.predicate` on the proposal artifact was killed by `artifact-frontmatter-equals` +mapping *artifact not found* to `block: true` — it would block every ship in which no +proposal was produced. A `check.query` was then killed by the test suite: `ship.md` has **no +generic `ship:pre` gate dispatch at all**, only two hardcoded branches (`security` at +`ship.md:105`, `broken-windows` at `ship.md:155`), so a third gate of either kind would be +declared and never evaluated — strict mode would silently do nothing. The structural guards +in `tests/loop-hooks-ship-pre-e2e.test.cjs` pin that reality rather than express a +preference. Generalized: **at `ship:pre`, a capability cannot add a gate; it can only add a +window.** + +**D7 — A declined refactor is recorded in the existing broken-windows ledger.** The same +`appendWindow` path as D6, degrading to a note when that capability is absent. No second ledger. + +**D8 — `execute:post` gains a generic step-hook dispatch contract.** `execute-phase.md` +dispatches `execute:post` step hooks only where `ref.skill == "code-review"`, so a `ref.command` +step there is declared-but-never-run. The contract added mirrors the existing one in `ship.md`. +The code-review branch is left byte-identical; the generic contract handles the rest. + +## What stays OUTSIDE + +- **Choosing or applying the refactor.** The capability surfaces a proposal. It never edits + code, never picks a strategy, never auto-applies. Executing an accepted proposal remains the + executor's existing `refactor` commit type. +- **Cognitive complexity, Halstead, maintainability index.** One metric behind a seam. Adding a + second is a later, additive decision, not a reason to widen this one. +- **Non-JS/TS languages.** Unsupported extensions are reported as skipped, never scored. +- **Test files and generated output.** Excluded by default; branchy tests are not a defect. +- **Threading the touched-file set through `agents/gsd-executor.md`** (named in the issue's + scope list). Deriving it from a bounded `git diff` inside the CLI is deterministic and + testable and avoids the agent-size ripple. +- **Repo-wide or scheduled scanning.** The signal is strongest on exactly the files the phase + just touched; a periodic full-repo scan is decoupled from the change that caused the growth. + +## Consequences + +**Good.** Continuous, automatic refactoring pressure that does not depend on anyone +remembering. Slow creep becomes visible a phase before the absolute threshold would catch it. +Declined cleanups stay tracked instead of evaporating. Zero cost when off, and zero new +dependencies when on. + +**Bad, and accepted.** The metric is approximate by construction: biased against a flat +`switch` (SonarSource's well-known objection to raw cyclomatic complexity), blind to nesting +depth, and JS/TS-only. A rename reads as delete-plus-add and loses its anchor. Defaults will +need tuning — too low reads as refactor spam, which the linked issue names as the main +maintenance burden. The leak surface of a non-AST analyzer is real; it is handled by refusing +to emit a number rather than by emitting an approximate one. + +**A soft dependency, disclosed.** Strict mode can only *block* when the broken-windows +capability is installed and `workflow.windows_enforce` is on — two toggles, not one. Without +it, strict mode still records the proposal and says so, but cannot stop a ship. `requires: +["broken-windows"]` was rejected because it would force-install the ledger on advisory users +who never enable strict mode. + +**Risk we are deliberately holding.** D5 mitigates Goodhart exposure but does not eliminate it. +A team that turns on strict mode and treats proposals as chores to clear will get worse code +than one that leaves it advisory. The default and the documentation both push toward the latter. + +## Open questions + +1. Is 15 the right default threshold? SonarSource's default; ESLint's is 20; radon's rank C + starts at 11. Needs field data. +2. Should an accepted proposal auto-create a task in the next phase's plan, rather than relying + on the developer to act? Deferred — it would couple this capability to the planner. +3. Should cognitive complexity become the default once available, with cyclomatic as the + fallback? The seam allows it; the evidence to decide does not exist yet. + +## References + +- *The Pragmatic Programmer*, Topic 40 — "Refactoring". +- T. J. McCabe, "A Complexity Measure", IEEE TSE, 1976 — `V(G) = E − N + 2P`. +- G. Ann Campbell, "Cognitive Complexity" (SonarSource) — the readability critique of raw + cyclomatic complexity that motivates D2's seam and the flat-`switch` caveat. +- ESLint `complexity` rule — the source of D3's strictly-greater `max` semantics. +- Martin Fowler, *Refactoring* and "Code Smell" — a threshold crossing is a signal to look + closer, not a verdict; the reason D5 surfaces rather than blocks. +- `docs/reference/capability-manifest.md`, `docs/reference/gate-predicates.md`. diff --git a/docs/adr/README.md b/docs/adr/README.md index 59258dd1c..413a1de3d 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -268,6 +268,7 @@ Decided in principle, not yet ratified. Do not cite as settled architecture. | [ADR-1213](1213-capability-state-writer.md) | Capability write side — the Capability State Writer | Proposed | — | | [ADR-1606](1606-prohibition-enforcement-verify-seam.md) | prohibition-enforcement verify-time seam | Proposed | — | | [ADR-1671](1671-dynamic-context-management-platform.md) | Dynamic context management platform | Proposed | — | +| [ADR-1953](1953-complexity-triggered-refactor.md) | Complexity-triggered refactor — the loop measures the entropy it just added | Proposed | — | | [ADR-3128](3128-adaptive-runtime-evidence.md) | Adaptive runtime evidence for GSD Debug | Proposed | — | ### Superseded, Retired, and Legacy diff --git a/docs/how-to/act-on-a-refactor-proposal.md b/docs/how-to/act-on-a-refactor-proposal.md new file mode 100644 index 000000000..b503384d5 --- /dev/null +++ b/docs/how-to/act-on-a-refactor-proposal.md @@ -0,0 +1,119 @@ +# How to catch complexity before it compounds + +**Goal:** Turn on the complexity-triggered refactor hook, understand the proposals it surfaces after a phase, and dispose of each one — so a function that grew a branch per phase gets refactored while it is still one function, instead of becoming a rewrite nobody noticed accumulating. + +**Prerequisites:** A GSD project on v1.10.0 or later. Nothing else — the hook has no external dependency and needs no indexer. It is **off by default**; a project that never enables it is completely unaffected. + +For what the metric measures, why the anchor moves only on disposition, and the metric's known biases, see [Complexity-Triggered Refactor](../FEATURES.md#159-complexity-triggered-refactor) and [ADR-1953](../adr/1953-complexity-triggered-refactor.md). This guide covers only how to *use* it. + +--- + +## Turn it on + +```bash +gsd config-set refactor.trigger_enabled true +``` + +That is the whole setup. From the next `/gsd-execute-phase`, an `execute:post` step measures the files that phase touched and writes a proposal only if something crosses a line. Nothing else about the loop changes: the hook never edits code, never picks a refactor, and never blocks. + +To check it without running a phase: + +```bash +node gsd-tools.cjs refactor evaluate --phase 3 --raw +``` + +--- + +## Read a proposal + +A triggered proposal lands at `.planning/phases//-REFACTOR.md`. It names one **target** — the hotspot — and lists every other candidate it found. + +Two numbers matter, and they answer different questions: + +- **`score`** — the function's absolute complexity right now. It triggered because it exceeds `refactor.complexity_threshold` (default 15). Answers *"is this function too complex?"* +- **`delta`** — growth over the function's **anchor**, the score recorded the last time you consciously decided about it. It triggered because it exceeds `refactor.complexity_jump_delta` (default 5). Answers *"has this been quietly creeping?"* + +A proposal can carry both reasons. The delta is the one worth reading closely: because the anchor does not move on its own, a function gaining two points per phase accumulates against it and trips the delta a phase or more *before* the absolute threshold would have caught it. That gap is the entire point of the second number. + +Both thresholds are **strictly greater** — a score of exactly 15 against a threshold of 15 does not trigger. This matches ESLint's `complexity: {max: N}`. + +--- + +## Act on it + +A proposal stays untriaged until you disposition it. There are exactly two ways, and **both** clear it: + +```bash +node gsd-tools.cjs refactor accept --phase 3 +node gsd-tools.cjs refactor decline --phase 3 --reason "flat dispatch table — branchy by construction, not a hotspot" +``` + +**Accept** when you intend to refactor. The anchor re-anchors to the function's current score, so after you do the work the next evaluation measures growth from the new, lower baseline. + +**Decline** when the complexity is justified. The reason is required and recorded. The anchor re-anchors to the *current* score — you have consciously accepted this much complexity, so the delta clock restarts from here rather than nagging you every phase about growth you already signed off. + +> **The score improving does not clear a proposal — only a disposition does.** This is deliberate. If clearing required the number to go down, the cheapest way to satisfy it would be splitting one coherent function into two incoherent ones: identical total complexity, worse cohesion. The gate asks whether you decided, not whether the metric moved. + +To see what is outstanding across the project: + +```bash +node gsd-tools.cjs refactor status +``` + +--- + +## Tune the thresholds + +If proposals feel like noise, raise the threshold rather than turning the hook off: + +```bash +gsd config-set refactor.complexity_threshold 20 # ESLint's own default +gsd config-set refactor.complexity_jump_delta 8 +``` + +Defaults are 15 (SonarSource's default) and 5. For reference, radon's rank C — "moderate, slightly complex" — begins at 11, and ESLint's `complexity` rule defaults to 20. There is no universally correct number; start at the default and raise it once you have seen a few proposals you disagreed with. + +--- + +## Make it block before ship + +Advisory mode surfaces proposals and tracks nothing. To make an untriaged proposal a task that must be resolved before shipping, you need **two** settings, not one: + +```bash +gsd config-set refactor.trigger_strict true # 1. record proposals in the ledger +gsd config-set workflow.windows_enforce true # 2. make the ledger block /gsd-ship +``` + +Why two: `refactor.trigger_strict` records an untriaged proposal as an open `deviation` entry in the [broken-windows ledger](../FEATURES.md#158-broken-windows-ledger). The *blocking* is that capability's existing `ship:pre` gate, which is separately opt-in. Setting only the first gives you tracking without enforcement — which is a reasonable place to stop, but it will not stop a ship. + +If you enable only `refactor.trigger_strict`, every `refactor evaluate` that triggers reports a typed warning saying so, so you never learn about the enforcement gap by hitting it at ship time: + +```json +"warnings": [ + { + "reason": "refactor_strict_not_enforcing", + "message": "refactor.trigger_strict is on, but workflow.windows_enforce is off, so ship will not actually be blocked. Run: gsd config-set workflow.windows_enforce true" + } +] +``` + +If the broken-windows capability is not installed, strict mode still records the proposal locally and says so in its output (`ledger_recorded: false` with a note) — and the same `refactor_strict_not_enforcing` warning fires, with a message telling you to install the broken-windows capability first. It cannot block on its own. + +Dispositioning resolves the ledger entry automatically: `accept` marks it `fixed`, `decline` marks it `waived` with your reason attached. + +--- + +## When it stays quiet + +The hook is deliberately silent in several situations. If you expected a proposal and got none, check these before assuming it is broken — `--raw` reports the reason in every case: + +| You see | What happened | +|---|---| +| `refactor_no_touched_files` | The phase changed nothing the analyzer looks at. | +| `refactor_analyzer_unsupported` | The file's language or path is out of scope. Only `.js .cjs .mjs .ts .cts .mts` are analyzed; `tests/` and generated `gsd-core/bin/lib/` paths are excluded by design. | +| `refactor_analyzer_unparseable` | An unterminated string, template, or block comment. The analyzer **refuses to emit a score** it cannot defend rather than guessing — a silently wrong number is worse than none. | +| `refactor_git_unavailable` | Not a git repository, `git` missing, the call timed out, or the phase has no committed `PLAN.md` to anchor against. Degrades quietly, exit 0. | +| `refactor_file_unreadable` | One file could not be read, or resolved outside the project root. That file is skipped; the rest of the run continues. | +| Nothing at all, `below_threshold` | Everything the phase touched is under both lines. This is the normal, healthy case. | + +Two limits worth knowing up front. A **renamed function loses its anchor** — it reads as a delete plus an add, so it is evaluated against the absolute threshold only until it is dispositioned again. And the metric is **biased against a flat `switch`**: twelve readable cases score 13, which is why proposals are advisory and why `decline` takes a reason instead of demanding a code change. diff --git a/docs/reference/capability-matrix.md b/docs/reference/capability-matrix.md index 97944ad7b..6ac8b5f20 100644 --- a/docs/reference/capability-matrix.md +++ b/docs/reference/capability-matrix.md @@ -44,7 +44,7 @@ Core package and are stamped with the package version at release (per ADR-1244 D6). They are not subject to the consent or integrity-pin flow applied to third-party capabilities. -### Feature capabilities (role: feature) — 20 +### Feature capabilities (role: feature) — 21 Feature capabilities extend what the loop does — contributing research, planning, execution, verification, or ship artefacts at the loop extension @@ -67,6 +67,7 @@ points. | `nyquist` | feature | full | `>=1.6.0` | `verify:post` | step | first-party | | `pattern-mapper` | feature | full | `>=1.6.0` | `plan:pre` | step | first-party | | `profile-pipeline` | feature | full | `>=1.6.0` | — | — | first-party | +| `refactor-trigger` | feature | full | `>=1.10.0` | `execute:post` | step | first-party | | `research` | feature | standard | `>=1.6.0` | `plan:pre` | step | first-party | | `schema-gate` | feature | full | `>=1.6.0` | `plan:pre` | contribution | first-party | | `security` | feature | full | `>=1.6.0` | `plan:pre`, `verify:post`, `ship:pre` | step, contribution, gate | first-party | diff --git a/eslint.config.mjs b/eslint.config.mjs index eb63f1827..e851fd436 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -87,6 +87,9 @@ export default tseslint.config( 'gsd-core/bin/lib/code-review-flags.cjs', 'gsd-core/bin/lib/context-utilization.cjs', 'gsd-core/bin/lib/broken-windows.cjs', + 'gsd-core/bin/lib/complexity-trigger.cjs', + // issue #1953: tsc-generated runtime artifact — lint the src/refactor-trigger-command-router.cts source. + 'gsd-core/bin/lib/refactor-trigger-command-router.cjs', 'gsd-core/bin/lib/api-coverage.cjs', 'gsd-core/bin/lib/artifacts.cjs', 'gsd-core/bin/lib/assumption-delta.cjs', diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index cb543a721..5d03f63f3 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -3033,6 +3033,73 @@ const capabilities = { "handler": null } }, + "refactor-trigger": { + "id": "refactor-trigger", + "role": "feature", + "version": "1.10.0", + "title": "Complexity-triggered refactor", + "description": "Measures the complexity of the code a phase touched and, when a function crosses a configured threshold or jumps past its recorded anchor, surfaces a scoped refactor proposal at .planning/phases//-REFACTOR.md. Advisory by default — it never edits code and never blocks. Opt-in strict mode blocks /gsd-ship while a proposal is untriaged; a declined proposal is recorded in the broken-windows ledger when that capability is present. Operationalizes 'refactor early, refactor often' as continuous pressure instead of a thing you have to remember (issue #1953).", + "tier": "full", + "requires": [], + "engines": { + "gsd": ">=1.10.0" + }, + "runtimeCompat": { + "supported": [ + "*" + ], + "unsupported": [] + }, + "skills": [], + "agents": [], + "activationKey": "refactor.trigger_enabled", + "config": { + "refactor.trigger_enabled": { + "type": "boolean", + "default": false, + "description": "Enable the complexity-triggered refactor hook. When true, an execute:post step evaluates the complexity of the files the phase touched and writes a scoped refactor proposal if a function crosses refactor.complexity_threshold or jumps past refactor.complexity_jump_delta. Opt-in; when false the hook never runs. Issue #1953." + }, + "refactor.complexity_threshold": { + "type": "number", + "default": 15, + "description": "Absolute per-function complexity above which a refactor proposal is surfaced. Semantics match ESLint's `complexity: {max: N}` — the trigger is STRICTLY GREATER, so a score of exactly N does not trigger. Default 15 follows SonarSource's default; ESLint's own default is 20 and radon's rank C begins at 11. Raise it if proposals feel like noise." + }, + "refactor.complexity_jump_delta": { + "type": "number", + "default": 5, + "description": "Complexity growth above which a refactor proposal is surfaced even when the absolute threshold is not reached. Measured against the function's anchor — the score recorded the last time the function was consciously dispositioned — so it accumulates across phases and catches slow creep the absolute threshold would miss. Strictly greater, as with the threshold." + }, + "refactor.trigger_strict": { + "type": "boolean", + "default": false, + "description": "Record an untriaged refactor proposal as an open `deviation` entry in the broken-windows ledger, so it becomes a tracked task that must be resolved before ship. Off by default and deliberately so: a blocking complexity number is a metric an executor can satisfy by splitting one coherent function into two incoherent ones, so the entry clears on the proposal being DISPOSITIONED (gsd-tools refactor accept|decline), never on the score improving. Ship blocking is the broken-windows capability's existing ship:pre gate — enable it with workflow.windows_enforce. With broken-windows absent, strict mode still records the proposal locally and says so; it cannot block on its own. Advisory mode (the default) surfaces the same proposal and tracks nothing." + } + }, + "commands": [ + { + "family": "refactor", + "module": "refactor-trigger-command-router.cjs", + "router": "routeRefactorTriggerCommand" + } + ], + "hooks": [], + "steps": [ + { + "point": "execute:post", + "ref": { + "command": "refactor evaluate" + }, + "produces": [ + "REFACTOR.md" + ], + "consumes": [], + "when": "refactor.trigger_enabled", + "onError": "skip" + } + ], + "contributions": [], + "gates": [] + }, "research": { "id": "research", "role": "feature", @@ -4157,6 +4224,19 @@ const byLoopPoint = { ], "when": "workflow.code_review", "onError": "skip" + }, + { + "capId": "refactor-trigger", + "point": "execute:post", + "ref": { + "command": "refactor evaluate" + }, + "produces": [ + "REFACTOR.md" + ], + "consumes": [], + "when": "refactor.trigger_enabled", + "onError": "skip" } ], "contributions": [], @@ -4360,6 +4440,10 @@ const configKeys = { "review.models.opencode": "opencode", "workflow.pattern_mapper": "pattern-mapper", "profile-pipeline.enabled": "profile-pipeline", + "refactor.trigger_enabled": "refactor-trigger", + "refactor.complexity_threshold": "refactor-trigger", + "refactor.complexity_jump_delta": "refactor-trigger", + "refactor.trigger_strict": "refactor-trigger", "workflow.research": "research", "workflow.schema_push_detection": "schema-gate", "workflow.security_enforcement": "security", @@ -4688,6 +4772,30 @@ const configSchema = { "default": false, "description": "Enable the developer profiling pipeline commands (scan-sessions, extract-messages, profile-sample, write-profile, etc.)." }, + "refactor.trigger_enabled": { + "owner": "refactor-trigger", + "type": "boolean", + "default": false, + "description": "Enable the complexity-triggered refactor hook. When true, an execute:post step evaluates the complexity of the files the phase touched and writes a scoped refactor proposal if a function crosses refactor.complexity_threshold or jumps past refactor.complexity_jump_delta. Opt-in; when false the hook never runs. Issue #1953." + }, + "refactor.complexity_threshold": { + "owner": "refactor-trigger", + "type": "number", + "default": 15, + "description": "Absolute per-function complexity above which a refactor proposal is surfaced. Semantics match ESLint's `complexity: {max: N}` — the trigger is STRICTLY GREATER, so a score of exactly N does not trigger. Default 15 follows SonarSource's default; ESLint's own default is 20 and radon's rank C begins at 11. Raise it if proposals feel like noise." + }, + "refactor.complexity_jump_delta": { + "owner": "refactor-trigger", + "type": "number", + "default": 5, + "description": "Complexity growth above which a refactor proposal is surfaced even when the absolute threshold is not reached. Measured against the function's anchor — the score recorded the last time the function was consciously dispositioned — so it accumulates across phases and catches slow creep the absolute threshold would miss. Strictly greater, as with the threshold." + }, + "refactor.trigger_strict": { + "owner": "refactor-trigger", + "type": "boolean", + "default": false, + "description": "Record an untriaged refactor proposal as an open `deviation` entry in the broken-windows ledger, so it becomes a tracked task that must be resolved before ship. Off by default and deliberately so: a blocking complexity number is a metric an executor can satisfy by splitting one coherent function into two incoherent ones, so the entry clears on the proposal being DISPOSITIONED (gsd-tools refactor accept|decline), never on the score improving. Ship blocking is the broken-windows capability's existing ship:pre gate — enable it with workflow.windows_enforce. With broken-windows absent, strict mode still records the proposal locally and says so; it cannot block on its own. Advisory mode (the default) surfaces the same proposal and tracks nothing." + }, "workflow.research": { "owner": "research", "type": "boolean", @@ -6897,6 +7005,11 @@ const commandFamilies = { "module": "profile-pipeline-command-router.cjs", "router": "routeProfileSample" }, + "refactor": { + "capId": "refactor-trigger", + "module": "refactor-trigger-command-router.cjs", + "router": "routeRefactorTriggerCommand" + }, "scan-sessions": { "capId": "profile-pipeline", "module": "profile-pipeline-command-router.cjs", @@ -7027,6 +7140,7 @@ const _requiresGraph = { "pi": [], "profile-pipeline": [], "qwen": [], + "refactor-trigger": [], "research": [], "schema-gate": [], "security": [], diff --git a/gsd-core/references/loop-hook-dispatch.md b/gsd-core/references/loop-hook-dispatch.md index 66eb9c43a..7e1014aef 100644 --- a/gsd-core/references/loop-hook-dispatch.md +++ b/gsd-core/references/loop-hook-dispatch.md @@ -31,7 +31,7 @@ Inject `fragment.inline` verbatim into the context for the role named in `into` ### `step` -Dispatch the referenced unit: +Dispatch the referenced unit. Exactly one of `ref.skill`, `ref.agent`, or `ref.command` is set. - `ref.skill` present → dispatch via the Skill tool with skill id `gsd-`. - `ref.agent` present → dispatch via the Agent tool with `subagent_type` = `ref.agent`. @@ -42,8 +42,31 @@ Dispatch the referenced unit: ◆ Spawning ... (runs in a subagent — no output until it returns; expected, not a freeze) ``` +- `ref.command` present → **validate it IN-CONTEXT first, before any shell use.** It comes + from a capability manifest, which may be third-party. Check the value you read from + `activeHooks` against `^[a-z][a-z0-9-]*( [a-z][a-z0-9-]*)*$` yourself — **never** by pasting + it into a shell command to be tested there, because a value carrying a quote, `;`, + `` ` ``, `$(`, or a newline would terminate the assignment and run as its own statement + before any shell-side check could execute. A value that fails is a malformed manifest: + record a warning, skip that hook, continue to the next entry. Only a value that has passed + is run, with the phase number appended: + + ```bash + gsd_run ${ref.command} --phase "${PHASE_NUMBER}" --raw + ``` + Wait for the result before continuing to the next hook or the next step. +A `step` is **advisory by construction**: it never blocks or redirects the host workflow — +that is what a `gate` is for. Each dispatch is best-effort; on error record a warning and +continue, honoring `onError`. + +**A point whose workflow hand-rolls one `kind` does not implement this contract.** Several +host workflows historically matched a single hook (e.g. `execute:post` matched only +`ref.skill == "code-review"`), so any other step registered there was declared and silently +never run. When a workflow defers to this file, it dispatches **every** active `step` entry, +not one shape of one. + ### `gate` Evaluate `check` (one of `query`, `predicate`, or `agentVerdict`). Then honor `blocking`: diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index 0420b0cdc..1413b6247 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -1229,7 +1229,7 @@ If `section_manifest` is `null` or `"partial-wave"` is in its `included` list: r EXECUTE_POST_HOOKS_JSON=${EXECUTE_POST_HOOKS_JSON:-$(gsd_run loop render-hooks execute:post --raw)} ``` -Resolve active step hooks from `EXECUTE_POST_HOOKS_JSON` where `kind == "step"` and `ref.skill == "code-review"`. +Dispatch `kind == "step"` hooks per @gsd-core/references/loop-hook-dispatch.md. `ref.skill == "code-review"`: If no active code-review step hook exists: display "Code review skipped (code-review capability inactive)" and proceed to gate dispatch. diff --git a/src/complexity-trigger.cts b/src/complexity-trigger.cts new file mode 100644 index 000000000..8aac67bab --- /dev/null +++ b/src/complexity-trigger.cts @@ -0,0 +1,1091 @@ +/** + * Complexity-triggered refactor extension point — analyzer, evaluator, and + * baseline persistence (issue #1953). + * + * LEAF MODULE — imports ONLY: node:fs, node:path. No other src/ imports. + * + * Pipeline: analyzeSource (decision-point complexity per function, via + * stripLiterals to blank comments/string/regex content while preserving + * length and line numbers) -> evaluateCandidates (threshold/jump-delta + * scoring against a stable per-function baseline anchor) -> nextBaseline / + * reanchorBaseline (anchor bookkeeping; the anchor never advances on a + * plain evaluate — only an explicit disposition moves it, via + * reanchorBaseline) -> renderProposal / parseProposal (the on-disk + * `-REFACTOR.md` artifact) -> readBaseline / writeBaseline (I/O, atomic + * temp-then-rename, never throws). + * + * Authoritative surface: .gsd/phase/feat-1953-complexity-triggered-refactor/41-api-contract.md + * Executable spec: tests/complexity-trigger.test.cjs + * + * Out of scope for this module: git invocation, config reads, + * capability-activation checks, CLI arg parsing, and the ship-gate query — + * those live in their own modules (command router / ship gate, not this + * leaf module). + * + * Exports: + * Constants: BASELINE_FILE_NAME, PROPOSAL_SUFFIX, SCHEMA_VERSION, + * DEFAULTS, ANALYZABLE_EXTENSIONS, VERDICT, REASON (includes + * REFACTOR_FILE_UNREADABLE for an unreadable/confined-escape file) + * Pure: stripLiterals, analyzeSource, isAnalyzablePath, + * evaluateCandidates, nextBaseline, reanchorBaseline, + * renderProposal, parseProposal + * I/O: readBaseline, writeBaseline + */ + +import fs from 'node:fs'; +import path from 'node:path'; + +// ─── Constants ───────────────────────────────────────────────────────────── + +export const BASELINE_FILE_NAME = 'complexity-baseline.json'; // under .planning/ +export const PROPOSAL_SUFFIX = '-REFACTOR.md'; // ${PHASE_DIR}/${PADDED}-REFACTOR.md +export const SCHEMA_VERSION = 1; + +export const DEFAULTS = Object.freeze({ threshold: 15, jumpDelta: 5 }); + +export const ANALYZABLE_EXTENSIONS = Object.freeze( + ['.js', '.cjs', '.mjs', '.ts', '.cts', '.mts'], +); + +export const VERDICT = Object.freeze({ + TRIGGERED: 'triggered', + BELOW_THRESHOLD: 'below_threshold', + SKIPPED: 'skipped', +}); + +/** + * Frozen reason enum. Adding a code = three coordinated changes: this enum, + * the emitting call site, and the Object.keys(REASON).sort() lock test. + */ +export const REASON = Object.freeze({ + REFACTOR_OK: 'refactor_ok', + REFACTOR_DISABLED: 'refactor_disabled', + REFACTOR_NO_TOUCHED_FILES: 'refactor_no_touched_files', + REFACTOR_GIT_UNAVAILABLE: 'refactor_git_unavailable', + REFACTOR_ANALYZER_UNSUPPORTED: 'refactor_analyzer_unsupported', + REFACTOR_ANALYZER_UNPARSEABLE: 'refactor_analyzer_unparseable', + REFACTOR_FILE_UNREADABLE: 'refactor_file_unreadable', + REFACTOR_BASELINE_MALFORMED: 'refactor_baseline_malformed', + REFACTOR_BASELINE_WRITE_FAILED: 'refactor_baseline_write_failed', + REFACTOR_STRICT_NOT_ENFORCING: 'refactor_strict_not_enforcing', + REFACTOR_ARTIFACT_NOT_FOUND: 'refactor_artifact_not_found', + REFACTOR_ALREADY_DISPOSITIONED: 'refactor_already_dispositioned', + REFACTOR_DECLINE_REASON_EMPTY: 'refactor_decline_reason_empty', + REFACTOR_INVALID_PHASE: 'refactor_invalid_phase', + REFACTOR_USAGE: 'refactor_usage', +}); + +// ─── Types ───────────────────────────────────────────────────────────────── + +export interface FunctionScore { + name: string; + startLine: number; + endLine: number; + score: number; +} + +export type AnalyzedFile = + | { file: string; ok: true; method: 'decision-points'; functions: FunctionScore[] } + | { file: string; ok: false; reason: string }; + +export interface BaselineEntry { + score: number; + phase?: string; +} +export type Baseline = Record; // key: `${file}::${name}` + +export interface Candidate { + file: string; + name: string; + startLine: number; + score: number; + baseline: number | null; // null when the function has no baseline + delta: number | null; // null when baseline is null — NEVER `score` + reasons: Array<'threshold' | 'jump'>; // ordered ['threshold','jump'] when both +} + +export interface Evaluation { + verdict: (typeof VERDICT)[keyof typeof VERDICT]; + candidates: Candidate[]; + target: Candidate | null; // candidates[0] ?? null + skipped: Array<{ file: string; reason: string }>; + thresholdUsed: number; + jumpDeltaUsed: number; +} + +export interface Proposal { + schema_version: number; + status: 'proposed' | 'accepted' | 'declined'; + phase: string; + target_file: string; + target_function: string; + score: number; + baseline: number | null; + delta: number | null; + metric: 'decision-points'; + recorded_at: string; + resolved_at: string | null; + reason: string; + candidates: Candidate[]; +} + +// ─── stripLiterals ────────────────────────────────────────────────────────── + +/** + * Words after which a following `/` must be interpreted as the start of an + * expression (so a `/` there is a regex literal, never division). + */ +const REGEX_KEYWORD_CONTEXT_WORDS = new Set([ + 'return', 'typeof', 'instanceof', 'in', 'of', 'new', 'delete', 'void', + 'throw', 'case', 'do', 'else', 'yield', 'await', 'default', +]); + +function blankChar(ch: string): string { + return ch === '\n' ? '\n' : ' '; +} + +/** + * Replaces comment and literal *content* with spaces, preserving length and + * every newline so line numbers survive. Handles `//`, `/* *\/`, `'`, `"`, + * backtick templates (`${...}` interpolations are kept as code), regex + * literals (disambiguated from division by the preceding significant + * token), and backslash escapes. Unterminated string / template / block + * comment => { ok:false, reason: REFACTOR_ANALYZER_UNPARSEABLE }. + */ +export function stripLiterals( + source: string, +): { ok: true; stripped: string } | { ok: false; reason: string } { + const len = source.length; + const out: string[] = new Array(len); + let i = 0; + let lastTokenIsValue = false; + + interface TemplateFrame { state: 'literal' | 'interp'; braceDepth: number } + const templateStack: TemplateFrame[] = []; + + while (i < len) { + const top = templateStack[templateStack.length - 1]; + + if (top && top.state === 'literal') { + const ch = source[i]; + if (ch === '\\') { + out[i] = blankChar(ch); + i++; + if (i < len) { out[i] = blankChar(source[i]); i++; } + continue; + } + if (ch === '`') { + out[i] = ' '; + templateStack.pop(); + i++; + lastTokenIsValue = true; + continue; + } + if (ch === '$' && source[i + 1] === '{') { + out[i] = '$'; + out[i + 1] = '{'; + top.state = 'interp'; + top.braceDepth = 0; + i += 2; + lastTokenIsValue = false; + continue; + } + out[i] = blankChar(ch); + i++; + continue; + } + + const ch = source[i]; + + // Line comment. + if (ch === '/' && source[i + 1] === '/') { + while (i < len && source[i] !== '\n') { out[i] = ' '; i++; } + continue; + } + + // Block comment. + if (ch === '/' && source[i + 1] === '*') { + out[i] = ' '; out[i + 1] = ' '; i += 2; + let closed = false; + while (i < len) { + if (source[i] === '*' && source[i + 1] === '/') { + out[i] = ' '; out[i + 1] = ' '; i += 2; closed = true; break; + } + out[i] = blankChar(source[i]); i++; + } + if (!closed) return { ok: false, reason: REASON.REFACTOR_ANALYZER_UNPARSEABLE }; + lastTokenIsValue = false; + continue; + } + + // Single/double-quoted string. A raw (unescaped) newline before the + // closing quote is treated as unterminated (matches real JS syntax). + if (ch === '\'' || ch === '"') { + const quote = ch; + out[i] = ' '; i++; + let closed = false; + while (i < len) { + const c = source[i]; + if (c === '\n') break; + if (c === '\\') { + out[i] = ' '; i++; + if (i < len && source[i] !== '\n') { out[i] = ' '; i++; } + continue; + } + if (c === quote) { out[i] = ' '; i++; closed = true; break; } + out[i] = ' '; i++; + } + if (!closed) return { ok: false, reason: REASON.REFACTOR_ANALYZER_UNPARSEABLE }; + lastTokenIsValue = true; + continue; + } + + // Template literal open. + if (ch === '`') { + out[i] = ' '; + templateStack.push({ state: 'literal', braceDepth: 0 }); + i++; + continue; + } + + // Regex literal candidate — only when the preceding significant token + // indicates an expression context (never after a value-producing token). + if (ch === '/' && !lastTokenIsValue) { + const start = i; + out[i] = ' '; i++; + let inClass = false; + let closed = false; + while (i < len) { + const c = source[i]; + if (c === '\n') break; + if (c === '\\') { + out[i] = ' '; i++; + if (i < len && source[i] !== '\n') { out[i] = ' '; i++; } + continue; + } + if (c === '[') { inClass = true; out[i] = ' '; i++; continue; } + if (c === ']') { inClass = false; out[i] = ' '; i++; continue; } + if (c === '/' && !inClass) { out[i] = ' '; i++; closed = true; break; } + out[i] = ' '; i++; + } + if (closed) { + while (i < len && /[a-zA-Z]/.test(source[i])) { out[i] = ' '; i++; } + lastTokenIsValue = true; + continue; + } + // Not actually a regex literal (never closed) — fall back to + // treating the '/' as division and re-scan normally from there. + i = start; + out[i] = source[i]; i++; + lastTokenIsValue = false; + continue; + } + + // Template interpolation brace tracking (real code inside ${...}). + if (top && top.state === 'interp') { + if (ch === '{') { top.braceDepth++; out[i] = ch; i++; lastTokenIsValue = false; continue; } + if (ch === '}') { + if (top.braceDepth === 0) { + out[i] = '}'; + top.state = 'literal'; + i++; + lastTokenIsValue = false; + continue; + } + top.braceDepth--; + out[i] = ch; i++; lastTokenIsValue = true; continue; + } + } + + // Identifier / keyword. + if (/[A-Za-z_$]/.test(ch)) { + let j = i; + while (j < len && /[A-Za-z0-9_$]/.test(source[j])) j++; + const word = source.slice(i, j); + for (let k = i; k < j; k++) out[k] = source[k]; + lastTokenIsValue = !REGEX_KEYWORD_CONTEXT_WORDS.has(word); + i = j; + continue; + } + + // Numeric literal. + if (/[0-9]/.test(ch)) { + let j = i; + while (j < len && /[0-9a-fA-FxXoObB.]/.test(source[j])) j++; + for (let k = i; k < j; k++) out[k] = source[k]; + lastTokenIsValue = true; + i = j; + continue; + } + + if (ch === ')' || ch === ']') { + out[i] = ch; i++; lastTokenIsValue = true; continue; + } + + if (ch === '\n') { out[i] = '\n'; i++; continue; } + + out[i] = ch; i++; + if (!/\s/.test(ch)) lastTokenIsValue = false; + } + + if (templateStack.length > 0) { + return { ok: false, reason: REASON.REFACTOR_ANALYZER_UNPARSEABLE }; + } + + return { ok: true, stripped: out.join('') }; +} + +// ─── analyzeSource ────────────────────────────────────────────────────────── + +const DECISION_KEYWORDS = new Set(['if', 'for', 'while', 'do', 'case', 'catch']); +// Words that can textually precede `(...) {` without being a method/function +// definition — must be excluded from the method-shorthand heuristic so +// `if (x) {` / `switch (x) {` etc. are never mistaken for a function. +const NON_METHOD_WORDS = new Set([...DECISION_KEYWORDS, 'switch', 'with']); + +function findMatchingParen(s: string, openIdx: number): number { + let depth = 0; + for (let k = openIdx; k < s.length; k++) { + if (s[k] === '(') depth++; + else if (s[k] === ')') { depth--; if (depth === 0) return k; } + } + return -1; +} + +/** + * Attempts to skip a generic type-parameter list starting at `s[idx] === '<'` + * (e.g. ``, ``, ``, ``, nested `>`). Returns the index just past the matching top-level + * `>`, or -1 when the content is not shaped like one — either the brackets + * never balance, or an operator token that never appears inside a type + * parameter list (`&&`, `||`, `==`, `<=`, `>=`, arithmetic, a bare `?`) + * shows up first. This — plus the caller only ever committing to the skip + * when a `(` immediately follows the close — is what keeps a `<` used as a + * less-than comparison from being mistaken for generics: a real comparison + * either fails to balance before an unrelated bracket closes, contains one + * of the disallowed operator tokens, or is not immediately followed by a + * parameter list. + */ +function skipGenericParamList(s: string, idx: number): number { + const len = s.length; + let i = idx + 1; + const stack: string[] = ['>']; + while (i < len) { + const ch = s[i]; + const next = s[i + 1]; + if (ch === '=' && next === '>') { i += 2; continue; } // nested function-type arrow (e.g. a default's `(a:X)=>Y`) + if ( + (ch === '=' && next === '=') + || (ch === '<' && next === '=') + || (ch === '>' && next === '=') + || (ch === '&' && next === '&') + || (ch === '|' && next === '|') + || ch === '+' || ch === '*' || ch === '%' || ch === '!' + || ch === '?' || ch === ';' || ch === '/' || ch === '-' + ) return -1; + if (ch === '<') { stack.push('>'); i++; continue; } + if (ch === '(') { stack.push(')'); i++; continue; } + if (ch === '[') { stack.push(']'); i++; continue; } + if (ch === '{') { stack.push('}'); i++; continue; } + if (ch === '>' || ch === ')' || ch === ']' || ch === '}') { + if (stack.length === 0 || stack[stack.length - 1] !== ch) return -1; + stack.pop(); + i++; + if (stack.length === 0) return i; + continue; + } + i++; + } + return -1; +} + +/** + * Attempts to skip a return-type / type-predicate annotation starting at + * `s[idx] === ':'` (e.g. `: Promise`, `: Record`, `: A | + * B`, `: { a: number }`, `: string[]`, `: x is Foo`). Returns the index of + * the terminator that follows — a function-body `{`, an arrow `=>`, or a + * statement-ending `;` — never consuming the terminator itself, or -1 when + * the content is not shaped like a type. `expectStart` tracks whether a + * fresh type token may begin here so an object-type literal's own `{`/`}` + * (`: { a: number }`) is never mistaken for the function body, and an + * unexpected/unmatched closing bracket (e.g. the real end of an enclosing + * `switch`/ternary this `:` was actually a member of, not a return type) + * aborts rather than guesses. + */ +function skipTypeExpr(s: string, idx: number): number { + const len = s.length; + let i = idx + 1; + const stack: string[] = []; + let expectStart = true; + while (i < len) { + const ch = s[i]; + if (/\s/.test(ch)) { i++; continue; } + const next = s[i + 1]; + if (stack.length === 0) { + if (ch === ';') return i; + if (ch === '=' && next === '>') return i; + if (ch === '{' && !expectStart) return i; + if (ch === ',' && !expectStart) return i; + if (ch === '?') return -1; // a bare top-level '?' means this was a ternary, not a return type + } + if (ch === '=' && next === '>') { i += 2; expectStart = true; continue; } // nested function-type arrow + if ( + (ch === '=' && next === '=') + || (ch === '<' && next === '=') + || (ch === '>' && next === '=') + || (ch === '&' && next === '&') + || (ch === '|' && next === '|') + || ch === '+' || ch === '*' || ch === '%' || ch === '!' + ) return -1; + if (ch === '<') { stack.push('>'); i++; expectStart = true; continue; } + if (ch === '(') { stack.push(')'); i++; expectStart = true; continue; } + if (ch === '[') { stack.push(']'); i++; expectStart = true; continue; } + if (ch === '{') { stack.push('}'); i++; expectStart = true; continue; } + if (ch === '>' || ch === ')' || ch === ']' || ch === '}') { + if (stack.length === 0 || stack[stack.length - 1] !== ch) return -1; + stack.pop(); + i++; + expectStart = false; + continue; + } + if (ch === '|' || ch === '&' || ch === ',' || ch === '.' || ch === ':') { + i++; expectStart = true; continue; + } + if (/[A-Za-z0-9_$]/.test(ch)) { + let j = i; + while (j < len && /[A-Za-z0-9_$]/.test(s[j])) j++; + i = j; + expectStart = false; + continue; + } + i++; + } + return -1; +} + +function newlinesBetween(s: string, from: number, to: number): number { + let n = 0; + for (let k = from; k < to; k++) if (s[k] === '\n') n++; + return n; +} + +/** Best-effort name inference for an arrow function from `NAME = ` or `NAME: ` immediately preceding it. */ +function inferArrowName(s: string, pos: number): string { + let j = pos - 1; + while (j >= 0 && /\s/.test(s[j])) j--; + const isPlainEquals = j >= 0 && s[j] === '=' + && s[j - 1] !== '=' && s[j - 1] !== '!' && s[j - 1] !== '<' && s[j - 1] !== '>'; + const isPropColon = j >= 0 && s[j] === ':'; + if (!isPlainEquals && !isPropColon) return ''; + j--; + while (j >= 0 && /\s/.test(s[j])) j--; + const end = j + 1; + while (j >= 0 && /[A-Za-z0-9_$]/.test(s[j])) j--; + return s.slice(j + 1, end); +} + +type FunctionFrame = + | { kind: 'function'; name: string; startLine: number; score: number; mode: 'brace' } + | { kind: 'function'; name: string; startLine: number; score: number; mode: 'expr'; entryDepth: number }; +type BlockFrame = { kind: 'block' }; +type ScanFrame = FunctionFrame | BlockFrame; + +function scanFunctions(stripped: string): FunctionScore[] { + const len = stripped.length; + let i = 0; + let line = 1; + let depth = 0; + const stack: ScanFrame[] = []; + const results: FunctionScore[] = []; + + function currentFunctionFrame(): FunctionFrame | null { + for (let k = stack.length - 1; k >= 0; k--) { + const f = stack[k]; + if (f.kind === 'function') return f; + } + return null; + } + + function bump(): void { + const f = currentFunctionFrame(); + if (f) f.score += 1; + } + + function closeExprFramesIfNeeded(atDepth: number): void { + for (;;) { + const top = stack[stack.length - 1]; + if (top && top.kind === 'function' && top.mode === 'expr' && top.entryDepth === atDepth) { + stack.pop(); + results.push({ name: top.name, startLine: top.startLine, endLine: line, score: top.score }); + } else { + break; + } + } + } + + while (i < len) { + const ch = stripped[i]; + + if (ch === '\n') { line++; i++; continue; } + if (/\s/.test(ch)) { i++; continue; } + + if (ch === '&' && stripped[i + 1] === '&') { bump(); i += 2; continue; } + if (ch === '|' && stripped[i + 1] === '|') { bump(); i += 2; continue; } + if (ch === '?') { + if (stripped[i + 1] === '.') { i += 2; continue; } + if (stripped[i + 1] === '?') { i += 2; continue; } + bump(); i += 1; continue; + } + + if (ch === '(') { + const closeIdx = findMatchingParen(stripped, i); + if (closeIdx !== -1) { + let k = closeIdx + 1; + while (k < len && /\s/.test(stripped[k])) k++; + if (stripped[k] === ':') { + const afterType = skipTypeExpr(stripped, k); + if (afterType !== -1) { + k = afterType; + while (k < len && /\s/.test(stripped[k])) k++; + } + } + if (stripped[k] === '=' && stripped[k + 1] === '>') { + const name = inferArrowName(stripped, i); + const startLine = line; + let bodyStart = k + 2; + while (bodyStart < len && /\s/.test(stripped[bodyStart])) bodyStart++; + const nl = newlinesBetween(stripped, i, bodyStart); + if (stripped[bodyStart] === '{') { + line += nl; + stack.push({ kind: 'function', name, startLine, score: 1, mode: 'brace' }); + depth++; + i = bodyStart + 1; + continue; + } + line += nl; + stack.push({ kind: 'function', name, startLine, score: 1, mode: 'expr', entryDepth: depth }); + i = bodyStart; + continue; + } + } + depth++; i++; continue; + } + if (ch === '[') { depth++; i++; continue; } + if (ch === ')' || ch === ']') { + closeExprFramesIfNeeded(depth); + depth--; i++; continue; + } + if (ch === '{') { + stack.push({ kind: 'block' }); + depth++; i++; continue; + } + if (ch === '}') { + closeExprFramesIfNeeded(depth); + depth--; + const top = stack.pop(); + if (top && top.kind === 'function' && top.mode === 'brace') { + results.push({ name: top.name, startLine: top.startLine, endLine: line, score: top.score }); + } + i++; continue; + } + if (ch === ';' || ch === ',') { + closeExprFramesIfNeeded(depth); + i++; continue; + } + + if (/[A-Za-z_$]/.test(ch)) { + let j = i; + while (j < len && /[A-Za-z0-9_$]/.test(stripped[j])) j++; + const word = stripped.slice(i, j); + const wordLine = line; + + if (word === 'function') { + let k = j; + while (k < len && /\s/.test(stripped[k])) k++; + if (stripped[k] === '*') { k++; while (k < len && /\s/.test(stripped[k])) k++; } + let name = ''; + if (k < len && /[A-Za-z_$]/.test(stripped[k])) { + let m = k; + while (m < len && /[A-Za-z0-9_$]/.test(stripped[m])) m++; + name = stripped.slice(k, m); + k = m; + while (k < len && /\s/.test(stripped[k])) k++; + } + if (stripped[k] === '<') { + const afterGenerics = skipGenericParamList(stripped, k); + if (afterGenerics !== -1) { + let kk = afterGenerics; + while (kk < len && /\s/.test(stripped[kk])) kk++; + if (stripped[kk] === '(') k = kk; // only commit if genuinely followed by params + } + } + if (stripped[k] === '(') { + const closeIdx = findMatchingParen(stripped, k); + if (closeIdx !== -1) { + let p = closeIdx + 1; + while (p < len && /\s/.test(stripped[p])) p++; + if (stripped[p] === ':') { + const afterType = skipTypeExpr(stripped, p); + if (afterType !== -1) { + p = afterType; + while (p < len && /\s/.test(stripped[p])) p++; + } + } + if (stripped[p] === '{') { + line += newlinesBetween(stripped, i, p); + stack.push({ kind: 'function', name, startLine: wordLine, score: 1, mode: 'brace' }); + depth++; + i = p + 1; + continue; + } + } + } + i = j; + continue; + } + + if (NON_METHOD_WORDS.has(word)) { + if (DECISION_KEYWORDS.has(word)) bump(); + i = j; + continue; + } + + { + let k = j; + while (k < len && /\s/.test(stripped[k])) k++; + if (stripped[k] === '<') { + const afterGenerics = skipGenericParamList(stripped, k); + if (afterGenerics !== -1) { + let kk = afterGenerics; + while (kk < len && /\s/.test(stripped[kk])) kk++; + if (stripped[kk] === '(') k = kk; // only commit if genuinely followed by params + } + } + if (stripped[k] === '(') { + const closeIdx = findMatchingParen(stripped, k); + if (closeIdx !== -1) { + let p = closeIdx + 1; + while (p < len && /\s/.test(stripped[p])) p++; + if (stripped[p] === ':') { + const afterType = skipTypeExpr(stripped, p); + if (afterType !== -1) { + p = afterType; + while (p < len && /\s/.test(stripped[p])) p++; + } + } + if (stripped[p] === '{') { + line += newlinesBetween(stripped, i, p); + stack.push({ kind: 'function', name: word, startLine: wordLine, score: 1, mode: 'brace' }); + depth++; + i = p + 1; + continue; + } + } + } else if (stripped[k] === '=' && stripped[k + 1] === '>') { + const name = inferArrowName(stripped, i); + let bodyStart = k + 2; + while (bodyStart < len && /\s/.test(stripped[bodyStart])) bodyStart++; + const nl = newlinesBetween(stripped, i, bodyStart); + if (stripped[bodyStart] === '{') { + line += nl; + stack.push({ kind: 'function', name, startLine: wordLine, score: 1, mode: 'brace' }); + depth++; + i = bodyStart + 1; + continue; + } + line += nl; + stack.push({ kind: 'function', name, startLine: wordLine, score: 1, mode: 'expr', entryDepth: depth }); + i = bodyStart; + continue; + } + } + + i = j; + continue; + } + + i++; + } + + closeExprFramesIfNeeded(depth); + return results; +} + +/** + * Normalizes CRLF -> LF *before* analysis so scores and line numbers are + * identical either way. Base score 1 per function, +1 for each of: if, + * else-if (the if, not the else), for, for..of, for..in, while, do, case + * (not default), catch, &&, ||, ?:. Word-boundary matched. Not counted: + * ?., ??, bare else, default:. Decision points are attributed to the + * innermost enclosing function; top-level code belongs to no function. + */ +export function analyzeSource( + source: string, +): { ok: true; method: 'decision-points'; functions: FunctionScore[] } | { ok: false; reason: string } { + const normalized = source.replace(/\r\n/g, '\n'); + const stripped = stripLiterals(normalized); + if (!stripped.ok) return { ok: false, reason: stripped.reason }; + return { ok: true, method: 'decision-points', functions: scanFunctions(stripped.stripped) }; +} + +// ─── isAnalyzablePath ─────────────────────────────────────────────────────── + +/** + * Normalizes separators unconditionally (never via path.sep). True iff the + * extension is analyzable and the path is not under tests/ or + * gsd-core/bin/lib/. + */ +export function isAnalyzablePath(relPath: string): boolean { + const normalized = relPath.replace(/\\/g, '/'); + const ext = path.posix.extname(normalized); + if (!ANALYZABLE_EXTENSIONS.includes(ext)) return false; + if (normalized === 'tests' || normalized.startsWith('tests/')) return false; + if (normalized === 'gsd-core/bin/lib' || normalized.startsWith('gsd-core/bin/lib/')) return false; + return true; +} + +// ─── evaluateCandidates ───────────────────────────────────────────────────── + +function coercePositiveNumber(v: unknown, fallback: number): number { + const n = typeof v === 'number' ? v : Number(v); + return Number.isFinite(n) && n > 0 ? n : fallback; +} + +export function evaluateCandidates(input: { + analyzed: AnalyzedFile[]; + baseline: Baseline; + threshold?: unknown; + jumpDelta?: unknown; +}): Evaluation { + const thresholdUsed = coercePositiveNumber(input.threshold, DEFAULTS.threshold); + const jumpDeltaUsed = coercePositiveNumber(input.jumpDelta, DEFAULTS.jumpDelta); + + const candidates: Candidate[] = []; + const skipped: Array<{ file: string; reason: string }> = []; + + for (const entry of input.analyzed) { + if (!entry.ok) { + skipped.push({ file: entry.file, reason: entry.reason }); + continue; + } + for (const fn of entry.functions) { + const key = `${entry.file}::${fn.name}`; + const baselineEntry = input.baseline[key]; + const baselineScore = baselineEntry ? baselineEntry.score : null; + const reasons: Array<'threshold' | 'jump'> = []; + if (fn.score > thresholdUsed) reasons.push('threshold'); + let delta: number | null = null; + if (baselineScore !== null) { + delta = fn.score - baselineScore; + if (delta > jumpDeltaUsed) reasons.push('jump'); + } + if (reasons.length > 0) { + candidates.push({ + file: entry.file, + name: fn.name, + startLine: fn.startLine, + score: fn.score, + baseline: baselineScore, + delta, + reasons, + }); + } + } + } + + candidates.sort((a, b) => { + if (b.score !== a.score) return b.score - a.score; + const ad = a.delta === null ? -Infinity : a.delta; + const bd = b.delta === null ? -Infinity : b.delta; + if (bd !== ad) return bd - ad; + if (a.file !== b.file) return a.file < b.file ? -1 : 1; + if (a.name !== b.name) return a.name < b.name ? -1 : 1; + return a.startLine - b.startLine; + }); + + return { + verdict: candidates.length > 0 ? VERDICT.TRIGGERED : VERDICT.BELOW_THRESHOLD, + candidates, + target: candidates[0] ?? null, + skipped, + thresholdUsed, + jumpDeltaUsed, + }; +} + +// ─── Baseline anchor bookkeeping ──────────────────────────────────────────── + +/** + * Anchor semantics (see 41-api-contract.md "Anchor semantics" — corrected + * 2026-08-09): for each successfully analyzed function, no prior entry + * inserts an anchor at the current score (first observation); a prior + * entry carries forward UNCHANGED. The anchor never advances on a plain + * evaluate, whether or not the function triggered — only reanchorBaseline + * (called by an explicit disposition) moves it. + */ +export function nextBaseline( + prev: Baseline, + analyzed: AnalyzedFile[], + opts: { analyzedFiles?: string[]; phase?: string } = {}, +): Baseline { + const next: Baseline = {}; + const seenKeys = new Set(); + + for (const entry of analyzed) { + if (!entry.ok) continue; + for (const fn of entry.functions) { + const key = `${entry.file}::${fn.name}`; + seenKeys.add(key); + const existing = prev[key]; + if (existing) { + next[key] = existing; + } else { + next[key] = opts.phase ? { score: fn.score, phase: opts.phase } : { score: fn.score }; + } + } + } + + const analyzedFileSet = new Set(opts.analyzedFiles ?? []); + for (const [key, entry] of Object.entries(prev)) { + if (seenKeys.has(key)) continue; + const file = key.slice(0, key.lastIndexOf('::')); + if (analyzedFileSet.has(file)) continue; // function no longer exists -> prune + next[key] = entry; // file untouched this run -> never prune + } + + return next; +} + +/** Moves the anchor for `key` to `score` — accept re-anchors to the post-refactor score, decline re-anchors to the current (higher) score. */ +export function reanchorBaseline( + prev: Baseline, + key: string, + score: number, + opts: { phase?: string } = {}, +): Baseline { + return { + ...prev, + [key]: opts.phase ? { score, phase: opts.phase } : { score }, + }; +} + +// ─── Proposal render/parse ────────────────────────────────────────────────── + +const PROPOSAL_JSON_FENCE_OPEN = '````json'; +const PROPOSAL_JSON_FENCE_CLOSE = '````'; + +export function renderProposal(p: Proposal): string { + const fm = [ + '---', + `schema_version: ${p.schema_version}`, + `status: ${p.status}`, + `phase: ${p.phase}`, + `target_file: ${p.target_file}`, + `target_function: ${p.target_function}`, + `score: ${p.score}`, + `baseline: ${p.baseline === null ? 'null' : p.baseline}`, + `delta: ${p.delta === null ? 'null' : p.delta}`, + `metric: ${p.metric}`, + `recorded_at: ${p.recorded_at}`, + `resolved_at: ${p.resolved_at === null ? 'null' : p.resolved_at}`, + `reason: ${JSON.stringify(p.reason)}`, + '---', + '', + ].join('\n'); + + const header = [ + `# Refactor proposal: ${p.target_file}::${p.target_function}`, + '', + `Score ${p.score}${p.baseline !== null ? ` (baseline ${p.baseline}, delta ${p.delta})` : ''} — status: ${p.status}.`, + '', + ].join('\n'); + + const jsonBlock = [ + PROPOSAL_JSON_FENCE_OPEN, + JSON.stringify(p.candidates, null, 2), + PROPOSAL_JSON_FENCE_CLOSE, + '', + ].join('\n'); + + return [fm, header, jsonBlock].join('\n'); +} + +/** + * Parses a rendered proposal back into its IR. Frontmatter is the fast + * scalar path; the JSON block is authoritative for candidates. Fails + * closed (returns null) on any structural drift rather than guessing. + */ +export function parseProposal(text: string): Proposal | null { + try { + if (!text.startsWith('---\n') && !text.startsWith('---\r\n')) return null; + const headerEnd = text.startsWith('---\r\n') ? 5 : 4; + const closeIdx = text.indexOf('\n---', headerEnd); + if (closeIdx === -1) return null; + const yamlBody = text.slice(headerEnd, closeIdx); + const fm: Record = {}; + for (const rawLine of yamlBody.split(/\r?\n/)) { + const line = rawLine.replace(/\r$/, ''); + if (line.trim() === '') continue; + const m = line.match(/^([a-zA-Z0-9_]+):\s*(.*)$/); + if (!m) return null; + fm[m[1]] = m[2].trim(); + } + + const jsonStart = text.indexOf(PROPOSAL_JSON_FENCE_OPEN); + if (jsonStart === -1) return null; + const jsonEnd = text.indexOf(PROPOSAL_JSON_FENCE_CLOSE, jsonStart + PROPOSAL_JSON_FENCE_OPEN.length); + if (jsonEnd === -1) return null; + const jsonText = text.slice(jsonStart + PROPOSAL_JSON_FENCE_OPEN.length, jsonEnd).trim(); + let candidates: unknown; + try { + candidates = JSON.parse(jsonText); + } catch { + return null; + } + if (!Array.isArray(candidates)) return null; + + const status = fm.status; + if (status !== 'proposed' && status !== 'accepted' && status !== 'declined') return null; + const schemaVersion = Number(fm.schema_version); + if (!Number.isInteger(schemaVersion)) return null; + const score = Number(fm.score); + if (!Number.isFinite(score)) return null; + const baseline = fm.baseline === 'null' ? null : Number(fm.baseline); + if (baseline !== null && !Number.isFinite(baseline)) return null; + const delta = fm.delta === 'null' ? null : Number(fm.delta); + if (delta !== null && !Number.isFinite(delta)) return null; + const resolvedAt = fm.resolved_at === 'null' || fm.resolved_at === undefined ? null : fm.resolved_at; + let reason = ''; + try { + const parsedReason: unknown = JSON.parse(fm.reason ?? '""'); + reason = typeof parsedReason === 'string' ? parsedReason : (fm.reason ?? ''); + } catch { + reason = fm.reason ?? ''; + } + + return { + schema_version: schemaVersion, + status, + phase: fm.phase ?? '', + target_file: fm.target_file ?? '', + target_function: fm.target_function ?? '', + score, + baseline, + delta, + metric: 'decision-points', + recorded_at: fm.recorded_at ?? '', + resolved_at: resolvedAt, + reason, + candidates: candidates as Candidate[], + }; + } catch { + return null; + } +} + +// ─── I/O: baseline persistence ────────────────────────────────────────────── + +interface FsDeps { + fs?: typeof fs; +} + +function baselinePath(planningDir: string): string { + return path.join(planningDir, BASELINE_FILE_NAME); +} + +const RENAME_RETRY_ERRNOS = new Set(['EPERM', 'EBUSY', 'EACCES']); +const RENAME_MAX_ATTEMPTS = 5; +const RENAME_BACKOFF_MS = 25; + +function renameWithRetry(fsImpl: typeof fs, tmp: string, target: string): void { + let lastErr: unknown; + for (let attempt = 0; attempt < RENAME_MAX_ATTEMPTS; attempt++) { + try { + fsImpl.renameSync(tmp, target); + return; + } catch (err: unknown) { + lastErr = err; + const code = (err && typeof err === 'object' && 'code' in err) + ? String((err as { code?: unknown }).code) + : ''; + if (code && RENAME_RETRY_ERRNOS.has(code) && attempt < RENAME_MAX_ATTEMPTS - 1) { + const delay = RENAME_BACKOFF_MS * Math.pow(2, attempt); + const start = Date.now(); + while (Date.now() - start < delay) { + // Busy-wait a very short time — transient locks usually clear quickly. + } + continue; + } + throw err; + } + } + throw lastErr; +} + +/** + * Degrades to { ok:false, baseline:{}, reason: REFACTOR_BASELINE_MALFORMED } + * on absent-but-unreadable, bad JSON, or a non-plain-object root — and + * never rewrites the file on a failed read. Missing file is + * { ok:true, baseline:{} } with no reason. Never throws. + */ +export function readBaseline( + planningDir: string, + deps: FsDeps = {}, +): { ok: boolean; baseline: Baseline; reason?: string } { + const fsImpl = deps.fs ?? fs; + const p = baselinePath(planningDir); + let raw: string; + try { + raw = fsImpl.readFileSync(p, 'utf8'); + } catch (e: unknown) { + const code = (e && typeof e === 'object' && 'code' in e) + ? String((e as { code?: unknown }).code) + : ''; + if (code === 'ENOENT') return { ok: true, baseline: {} }; + return { ok: false, baseline: {}, reason: REASON.REFACTOR_BASELINE_MALFORMED }; + } + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return { ok: false, baseline: {}, reason: REASON.REFACTOR_BASELINE_MALFORMED }; + } + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { + return { ok: false, baseline: {}, reason: REASON.REFACTOR_BASELINE_MALFORMED }; + } + return { ok: true, baseline: parsed as Baseline }; +} + +/** + * Writes `..tmp` then renames; on any failure unlinks the temp + * file and returns { ok:false, reason: REFACTOR_BASELINE_WRITE_FAILED }. + * Never throws. + */ +export function writeBaseline( + planningDir: string, + baseline: Baseline, + deps: FsDeps = {}, +): { ok: boolean; reason?: string } { + const fsImpl = deps.fs ?? fs; + const p = baselinePath(planningDir); + const tmp = `${p}.${process.pid}.tmp`; + + try { + if (!fsImpl.existsSync(planningDir)) { + fsImpl.mkdirSync(planningDir, { recursive: true }); + } + fsImpl.writeFileSync(tmp, JSON.stringify(baseline, null, 2), 'utf8'); + } catch { + try { fsImpl.unlinkSync(tmp); } catch { /* best-effort cleanup */ } + return { ok: false, reason: REASON.REFACTOR_BASELINE_WRITE_FAILED }; + } + + try { + renameWithRetry(fsImpl, tmp, p); + } catch { + try { fsImpl.unlinkSync(tmp); } catch { /* best-effort cleanup */ } + return { ok: false, reason: REASON.REFACTOR_BASELINE_WRITE_FAILED }; + } + + return { ok: true }; +} diff --git a/src/git-base-branch.cts b/src/git-base-branch.cts index cef69d005..76771b745 100644 --- a/src/git-base-branch.cts +++ b/src/git-base-branch.cts @@ -297,6 +297,113 @@ export function gitWorktreeInfoInternal( } } +// ─── Adapter 3: phase-start anchor + touched-file listing (issue #1953) ─────── + +/** + * Resolve the commit that ADDED `/*-PLAN.md` — the anchor commit + * marking when the phase began (see `.gsd/phase/feat-1953-complexity-triggered- + * refactor/42-router-contract.md`, "Touched-file anchor"). `phaseDir` is a + * project-relative path (backslashes are normalized unconditionally before + * building the pathspec, never via `path.sep` — matches the repo's + * cross-platform path-normalization convention). + * + * Bounded (`timeout: 15_000`), degrades to `null` on any failure or when no + * such commit exists (a phase never planned through git, a shallow clone). + * Never throws. + */ +export function phaseStartCommit( + cwd: string, + phaseDir: string, + execGit?: ExecGitFn +): string | null { + const git: ExecGitFn = execGit ?? execGitSeam; + try { + const normalizedPhaseDir = phaseDir.replace(/\\/g, '/'); + const pathspec = `${normalizedPhaseDir}/*-PLAN.md`; + const r = git( + ['log', '--format=%H', '--diff-filter=A', '-1', '--', pathspec], + { cwd, timeout: 15_000 } + ); + if (r.exitCode !== 0 || !r.stdout) return null; + const sha = r.stdout.trim(); + return sha || null; + } catch { + return null; + } +} + +/** + * Reject characters/sequences that have no legitimate use in a git revision + * expression reaching this module (a ref name, SHA, or `~N` / `^N` + * / `@{...}` navigation) but that a hostile `--since` value could use to + * confuse either the shell-out or a downstream reader: whitespace, ASCII + * control characters, and the option-shaped `?`, `*`, `[`, `\` characters + * that `git check-ref-format` also disallows in ref *names*. A leading `-` + * is rejected outright — that is the actual option-injection vector `--end- + * of-options` (below) already neutralizes, so this is belt-and-suspenders + * for older git. `..` is rejected because `sinceRef` is a single revision + * that this function itself turns into a range (`sinceRef..HEAD`); a + * `sinceRef` that already contains `..` can only produce a malformed or + * misleading range. A trailing `.lock` is rejected per `check-ref-format`. + * + * Deliberately NOT rejected: `~`, `^`, `:`, `@`, `{`, `}` — `check-ref- + * format` disallows these in a bare ref *name*, but this value is a git + * *revision expression*, and rejecting them would break entirely ordinary + * user input such as `HEAD~1`, `HEAD^`, or `main@{yesterday}`. None of + * these characters can reintroduce option parsing once `--end-of-options` + * is in effect, so allowing them costs nothing security-wise. + */ +function isSafeRevisionRef(ref: string): boolean { + if (ref === '') return false; + if (ref.startsWith('-')) return false; + if (/[\x00-\x1f\x7f ?*[\\]/.test(ref)) return false; + if (ref.includes('..')) return false; + if (ref.endsWith('.lock')) return false; + return true; +} + +/** + * List files changed between `sinceRef` and `HEAD`, NUL-delimited and + * quotepath-safe. Load-bearing details (see the router contract): + * - `-z` and `-c core.quotepath=false` avoid git's lossy quote-and-escape + * round-trip for non-ASCII paths; + * - splitting on `NUL` (never `\n`) tolerates a filename containing a real + * newline (git permits it); + * - `sinceRef` is validated by `isSafeRevisionRef` AND the revision-range + * argument is preceded by `--end-of-options`. A trailing `--` alone does + * NOT stop git from option-parsing an argument that appears BEFORE it — + * it only stops PATHSPEC interpretation of arguments AFTER it — so + * `--since '--output=/tmp/pwn'` would otherwise become the argument + * `--output=/tmp/pwn..HEAD`, which git accepts as an option and uses to + * redirect diff output to an attacker-chosen path. `--end-of-options` + * (git >= 2.24) is the correct fix: everything after it is parsed as a + * revision or path, never as an option, regardless of leading `-`. + * + * Bounded (`timeout: 15_000`), degrades to `null` when `sinceRef` fails + * validation or the underlying git call fails (non-zero exit, timeout, or + * spawn error) — never throws. An empty result set (no files changed + * between the two revisions) is a valid, non-null answer: `[]`. + */ +export function changedFilesSince( + cwd: string, + sinceRef: string, + execGit?: ExecGitFn +): string[] | null { + if (!isSafeRevisionRef(sinceRef)) return null; + const git: ExecGitFn = execGit ?? execGitSeam; + try { + const r = git( + ['-c', 'core.quotepath=false', 'diff', '--name-only', '-z', '--end-of-options', `${sinceRef}..HEAD`, '--'], + { cwd, timeout: 15_000 } + ); + if (r.exitCode !== 0) return null; + if (!r.stdout) return []; + return r.stdout.split('\0').filter((f) => f.length > 0); + } catch { + return null; + } +} + // ─── CLI entry point ────────────────────────────────────────────────────────── /** diff --git a/src/refactor-trigger-command-router.cts b/src/refactor-trigger-command-router.cts new file mode 100644 index 000000000..8c606ebc7 --- /dev/null +++ b/src/refactor-trigger-command-router.cts @@ -0,0 +1,923 @@ +'use strict'; +/** + * Refactor-trigger command router — CLI subcommand dispatcher for + * `gsd-tools refactor` (issue #1953). + * + * Follows `src/intel-command-router.cts` exactly: `routeHubCommandFamily`, + * `makeInvalidArgs` for validation failures, `output(value, raw)` for + * success, `_`-prefixed injection seams, lazy `require` of the heavy leaf + * module (`complexity-trigger.cjs`) and the adjacent modules + * (`git-base-branch.cjs`, `broken-windows.cjs`) inside the route function. + * + * Authoritative surfaces: + * .gsd/phase/feat-1953-complexity-triggered-refactor/42-router-contract.md + * .gsd/phase/feat-1953-complexity-triggered-refactor/41-api-contract.md + * + * `src/complexity-trigger.cts` stays a pure leaf (analyzer, evaluator, + * baseline persistence) — this module owns capability-activation gating, + * git invocation (via the `src/git-base-branch.cts` adapters), config + * reads, CLI arg parsing, phase-directory resolution, and the optional + * broken-windows ledger integration. It never duplicates the leaf's logic. + * + * Arg indexing: + * args[0] = 'refactor' (family — matched by dispatchCapabilityCommand) + * args[1] = subcommand (accept | decline | evaluate | status) + * args[2..] = flags (--phase, --since, --reason, --raw) + * + * Seams: `_complexity` (complexity-trigger.cjs), `_git` (git-base-branch.cjs), + * `_windows` (broken-windows.cjs — OPTIONAL; absent or a throwing `require` + * is the documented degrade path, never an error), `_core` (output capture). + * Production callers omit all four. + */ + +import path from 'node:path'; +import fs from 'node:fs'; +import { retryRenameSync } from './shell-command-projection.cjs'; + +// eslint-disable-next-line @typescript-eslint/no-require-imports +import io = require('./io.cjs'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +import commandRoutingHub = require('./command-routing-hub.cjs'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +import cjsCommandRouterAdapter = require('./cjs-command-router-adapter.cjs'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +import capabilityStateMod = require('./capability-state.cjs'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +import capabilityActivationMod = require('./capability-activation.cjs'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +import configLoaderMod = require('./config-loader.cjs'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +import phaseLocatorMod = require('./phase-locator.cjs'); +// eslint-disable-next-line @typescript-eslint/no-require-imports +import planningWorkspaceMod = require('./planning-workspace.cjs'); + +// Type-only: erased at emit, so pulling in the leaf modules' exact exported +// shapes here does not violate the "lazy require of the heavy module inside +// the route function" rule — only the runtime `require(...)` calls below are +// deferred into the handlers. +import type * as ComplexityTriggerModule from './complexity-trigger.cjs'; +import type { AnalyzedFile, Candidate, Evaluation, Proposal } from './complexity-trigger.cjs'; +import type * as GitBaseBranchModule from './git-base-branch.cjs'; +import type * as BrokenWindowsModule from './broken-windows.cjs'; +import type { Ledger, WindowEntry } from './broken-windows.cjs'; + +const { output } = io; +const { makeInvalidArgs } = commandRoutingHub; +const { routeHubCommandFamily } = cjsCommandRouterAdapter; +const { isCapabilityActive } = capabilityStateMod; +const { resolveConfigKey } = capabilityActivationMod; +const { loadConfig } = configLoaderMod; +const { findPhaseInternal, listMilestonePhaseDirs } = phaseLocatorMod; +const { planningDir } = planningWorkspaceMod; + +const CAPABILITY_ID = 'refactor-trigger'; + +// ─── Types ──────────────────────────────────────────────────────────────────── + +type ComplexityModule = typeof ComplexityTriggerModule; +type GitModule = typeof GitBaseBranchModule; +type WindowsModule = typeof BrokenWindowsModule; + +interface CoreModule { + output(value: unknown, raw: boolean): void; +} + +// Default CoreModule implementation. `_core` seam overrides this entirely +// for test injection (captures output calls without writing to real stdout). +const _defaultCore: CoreModule = { output }; + +interface RouteRefactorTriggerCommandOptions { + args: string[]; + cwd: string; + raw: boolean; + error: (message: string, reason?: string) => void; + /** Test seam: inject a mock complexity-trigger module. Defaults to the real module. */ + _complexity?: ComplexityModule; + /** Test seam: inject a mock git-base-branch module. Defaults to the real module. */ + _git?: GitModule; + /** Test seam: inject a mock broken-windows module. Defaults to the real module (optional capability). */ + _windows?: WindowsModule; + /** Test seam: inject a mock core module to capture output calls. Defaults to the real module. */ + _core?: CoreModule; +} + +// ─── Disabled response ────────────────────────────────────────────────────── + +function disabledResponse(): { disabled: true; message: string } { + return { + disabled: true, + message: 'refactor-trigger is not enabled. Enable with: gsd config-set refactor.trigger_enabled true', + }; +} + +// ─── Arg parsing ──────────────────────────────────────────────────────────── + +// Positive integer, optionally zero-padded, optionally with dotted decimal +// sub-phase segments (1, 01, 12, 3.1). Rejects 0, negative, letters, and +// whitespace — matches the router contract's "positive integer or decimal +// phase id" rule. +const PHASE_VALUE_RE = /^0*[1-9]\d*(?:\.\d+)*$/; + +interface FlagRead { + present: boolean; + value: string; + duplicated: boolean; + /** True when `--flag` was followed by another flag-shaped token (or nothing) instead of a value. */ + flagShaped: boolean; +} + +/** + * Reads a `--flag value` or `--flag=value` occurrence from `args`. Repeated + * occurrences are reported via `duplicated` (the last one wins in `value`, + * but callers should treat `duplicated` as invalid input). A token + * immediately following `--flag` that itself starts with `--` (or the flag + * being the last token) is NOT consumed as a value — `flagShaped` is set and + * `value` stays `''`, so `--phase --raw` never silently swallows `--raw`. + */ +function readFlag(args: string[], flag: string): FlagRead { + let present = false; + let value = ''; + let count = 0; + let flagShaped = false; + const eqPrefix = `${flag}=`; + for (let i = 0; i < args.length; i++) { + const a = args[i]; + if (a === flag) { + count++; + present = true; + const next = args[i + 1]; + if (next === undefined) { + value = ''; + } else if (next.startsWith('--')) { + value = ''; + flagShaped = true; + } else { + value = next; + flagShaped = false; + } + } else if (a.startsWith(eqPrefix)) { + count++; + present = true; + value = a.slice(eqPrefix.length); + flagShaped = false; + } + } + return { present, value, duplicated: count > 1, flagShaped }; +} + +/** + * Validate `--phase`: required, and a positive integer or decimal phase id. + * Empty, whitespace-only, duplicated, `--phase=`, `--phase==N`, or a + * flag-shaped value all fail. Missing entirely -> REFACTOR_USAGE (no value + * was ever offered); present but invalid -> REFACTOR_INVALID_PHASE (a value + * was given but rejected). Never throws. + */ +function requirePhaseArg( + args: string[], + complexity: ComplexityModule, + usage: string, +): { ok: true; phase: string } | { ok: false; result: unknown } { + const flag = readFlag(args, '--phase'); + if (!flag.present) { + return { ok: false, result: makeInvalidArgs('phase', usage, complexity.REASON.REFACTOR_USAGE) }; + } + const trimmed = flag.value.trim(); + if (flag.duplicated || flag.flagShaped || trimmed === '' || !PHASE_VALUE_RE.test(trimmed)) { + return { + ok: false, + result: makeInvalidArgs( + 'phase', + `Invalid --phase value: ${JSON.stringify(flag.value)}. ${usage}`, + complexity.REASON.REFACTOR_INVALID_PHASE, + ), + }; + } + return { ok: true, phase: trimmed }; +} + +// ─── Phase / path resolution ──────────────────────────────────────────────── + +interface ResolvedPhase { + /** Project-relative, POSIX-separated phase directory (e.g. ".planning/phases/03-x"). */ + phaseDir: string; + /** Zero-padded phase token (e.g. "03"), matching `${PADDED}-REFACTOR.md`. */ + padded: string; +} + +/** + * Resolve the on-disk phase directory for a validated `--phase` value. A + * phase that cannot be located on disk (never planned, wrong number, an + * archived milestone this call isn't scoped to) returns `null` — this is a + * distinct fact from a git failure (git is fine; the phase just doesn't + * exist), so callers report REFACTOR_INVALID_PHASE, never + * REFACTOR_GIT_UNAVAILABLE, for this case. + */ +function resolvePhaseDirForArg(cwd: string, phaseArg: string): ResolvedPhase | null { + const search = findPhaseInternal(cwd, phaseArg); + if (!search || search.found !== true) return null; + return { phaseDir: search.directory, padded: search.phase_number }; +} + +/** + * Resolve `relFile` (a repo-relative path, typically from `git diff + * --name-only`) against `cwd` and refuse a path that escapes the project + * root. Returns the absolute path, or `null` when it escapes. + * + * Two layers: the string-level `startsWith` check is a cheap first gate but + * confines only the SYMLINK's own path, not its target — a symlink + * committed in the repo (e.g. `src/evil.cts -> /etc/passwd`) has an + * in-tree path that passes the string check, and `fs.readFileSync` on it + * would follow the link and read outside the root. `lstatSync` (which does + * NOT follow symlinks, unlike `statSync`) is the second gate: anything that + * is not a regular file — a symlink most of all — is refused outright. + * Refusing rather than resolving-and-re-confining is deliberate: it is + * simpler and strictly stricter (a symlink whose target legitimately lives + * inside the root is still refused, which is an acceptable false positive + * for this analyzer). An `lstatSync` failure (ENOENT on a broken symlink, + * EACCES, a race) is treated the same as "not a regular file" — never + * propagated — so callers can keep using their existing null-means-skip + * convention (`REASON.REFACTOR_FILE_UNREADABLE`) uniformly. + */ +function resolveConfinedPath(cwd: string, relFile: string): string | null { + const root = path.resolve(cwd); + const resolved = path.resolve(root, relFile); + if (resolved !== root && !resolved.startsWith(root + path.sep)) return null; + try { + if (!fs.lstatSync(resolved).isFile()) return null; + } catch { + return null; + } + return resolved; +} + +function artifactFileName(padded: string, complexity: ComplexityModule): string { + return `${padded}${complexity.PROPOSAL_SUFFIX}`; +} + +function artifactAbsPath(cwd: string, phaseDir: string, padded: string, complexity: ComplexityModule): string { + return path.join(cwd, phaseDir, artifactFileName(padded, complexity)); +} + +function candidateKey(file: string, name: string): string { + return `${file}::${name}`; +} + +// ─── Atomic write (proposal + ledger) ─────────────────────────────────────── + +/** + * Atomic publish: write a sibling `.tmp.` file, then rename over the + * target via `retryRenameSync` (transient-Windows-lock-tolerant — matches + * `local/require-fs-op-fallback`, ADR-1703 Phase 6). On any failure, best- + * effort unlinks the temp file and rethrows. + */ +function writeTextAtomic(filePath: string, content: string): void { + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + const tmp = `${filePath}.${process.pid}.tmp`; + fs.writeFileSync(tmp, content, 'utf8'); + try { + retryRenameSync(tmp, filePath); + } catch (err) { + try { fs.unlinkSync(tmp); } catch { /* best-effort cleanup */ } + throw err; + } +} + +// ─── Config reads ─────────────────────────────────────────────────────────── + +/** + * Read `refactor.complexity_threshold` / `refactor.complexity_jump_delta` / + * `refactor.trigger_strict` via the same four-level precedence walk + * (`resolveConfigKey`) the capability-activation gate uses: loadConfig + * result -> workstream config.json -> root config.json -> registry + * `configSchema` default. Reading the frozen first-party registry directly + * is deliberate (not `capability-loader.cjs`'s `loadRegistry`): with + * `includeInstalled` omitted, `loadRegistry()` returns the exact same frozen + * object — refactor-trigger is first-party, so the overlay/consent machinery + * has nothing to add here and this avoids the extra indirection. + */ +function readEvalConfig(cwd: string): { threshold: unknown; jumpDelta: unknown; strict: boolean; windowsEnforce: boolean } { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const registry = require('./capability-registry.cjs') as Record; + const config = loadConfig(cwd); + const threshold = resolveConfigKey('refactor.complexity_threshold', { config, cwd, registry }).value; + const jumpDelta = resolveConfigKey('refactor.complexity_jump_delta', { config, cwd, registry }).value; + const strict = resolveConfigKey('refactor.trigger_strict', { config, cwd, registry }).value; + // Read via the SAME four-level precedence walk as the `refactor.*` keys + // above — reusing `resolveConfigKey`/`loadConfig`/the frozen registry + // rather than a second config reader. A missing key resolves to the + // registry default (false), matching "treat a missing key as falsy". + const windowsEnforce = resolveConfigKey('workflow.windows_enforce', { config, cwd, registry }).value; + return { threshold, jumpDelta, strict: Boolean(strict), windowsEnforce: Boolean(windowsEnforce) }; +} + +// ─── Strict-mode enforcement-gap warning (#1953) ──────────────────────────── + +const WINDOWS_ENFORCE_REMEDIATION = 'gsd config-set workflow.windows_enforce true'; + +/** + * `refactor.trigger_strict` only ever APPENDS a deviation window to the + * broken-windows ledger — a ship only actually stops when the separate + * `workflow.windows_enforce` toggle is also on. A user who enables only + * `refactor.trigger_strict` gets tracking with no enforcement and, absent + * this warning, no signal that this is the case. Fires strictly on TRIGGERED + * evaluates with strict mode on, mirroring the existing ledger-recording + * gate: either the broken-windows capability could not record the window at + * all (`ledgerRecorded === false` — the existing degrade path), or it did, + * but `workflow.windows_enforce` is falsy. Never fires with strict off. + */ +function strictNotEnforcingWarning( + complexity: ComplexityModule, + ledgerRecorded: boolean, + windowsEnforce: boolean, +): { reason: string; message: string } | null { + if (!ledgerRecorded) { + return { + reason: complexity.REASON.REFACTOR_STRICT_NOT_ENFORCING, + message: 'refactor.trigger_strict is on, but the broken-windows capability is unavailable, so ship will not ' + + `actually be blocked. Install the broken-windows capability, then run: ${WINDOWS_ENFORCE_REMEDIATION}`, + }; + } + if (!windowsEnforce) { + return { + reason: complexity.REASON.REFACTOR_STRICT_NOT_ENFORCING, + message: 'refactor.trigger_strict is on, but workflow.windows_enforce is off, so ship will not actually be ' + + `blocked. Run: ${WINDOWS_ENFORCE_REMEDIATION}`, + }; + } + return null; +} + +// ─── Broken-windows integration (strict mode; OPTIONAL) ───────────────────── + +function windowsLedgerPath(cwd: string, windows: WindowsModule): string { + return path.join(cwd, '.planning', windows.LEDGER_FILE_NAME); +} + +function readLedgerOrEmpty(cwd: string, windows: WindowsModule): Ledger | null { + const ledgerPath = windowsLedgerPath(cwd, windows); + try { + const raw = fs.readFileSync(ledgerPath, 'utf8'); + return windows.parseLedger(raw); + } catch (e: unknown) { + const code = (e && typeof e === 'object' && 'code' in e) ? String((e as { code?: unknown }).code) : ''; + if (code === 'ENOENT') return windows.emptyLedger(new Date().toISOString()); + return null; // unreadable/malformed — degrade rather than throw + } +} + +/** + * Structural identity match (#1953 defect 2): a deviation window's identity + * is the typed `phase`/`file`/`line` triple `appendWindow` already persists + * — never the human-readable `description` prose. A reworded description or + * a user editing `WINDOWS.md` prose can never break dedup. + */ +function findOpenDeviationEntry(ledger: Ledger, phase: string, file: string, line: number): WindowEntry | null { + return ledger.entries.find( + (e) => e.status === 'open' && e.kind === 'deviation' && e.phase === phase && e.file === file && e.line === line, + ) ?? null; +} + +/** + * Shared require-or-degrade + ledger-read boilerplate for + * `recordStrictWindow` / `resolveLedgerWindow` (#1953 defect 5a). `_windows` + * unavailable (module absent, or its `require` throws) or the ledger + * unreadable both degrade to a `{ ok: false, note }` result — never an + * error, never throws. + */ +function loadWindowsOrDegrade( + cwd: string, + windowsOverride: WindowsModule | undefined, +): { ok: true; windows: WindowsModule; ledger: Ledger } | { ok: false; note: string } { + let windows: WindowsModule; + try { + // eslint-disable-next-line @typescript-eslint/no-require-imports + windows = windowsOverride ?? (require('./broken-windows.cjs') as WindowsModule); + } catch { + return { ok: false, note: 'broken-windows capability unavailable — proposal recorded locally only' }; + } + const ledger = readLedgerOrEmpty(cwd, windows); + if (ledger === null) { + return { ok: false, note: 'broken-windows ledger unreadable — proposal recorded locally only' }; + } + return { ok: true, windows, ledger }; +} + +/** + * Strict-mode window append (step 9 of `evaluate`). Degrades to + * `{ recorded: false, note }` per `loadWindowsOrDegrade` — never an error, + * never throws. Idempotent: re-evaluating the same still-untriaged phase + * finds the existing OPEN entry (matched structurally on phase + target + * file/line) and does not append a second one. + */ +function recordStrictWindow( + cwd: string, + padded: string, + target: Candidate, + windowsOverride: WindowsModule | undefined, +): { recorded: boolean; note?: string } { + const loaded = loadWindowsOrDegrade(cwd, windowsOverride); + if (!loaded.ok) return { recorded: false, note: loaded.note }; + const { windows, ledger } = loaded; + + if (findOpenDeviationEntry(ledger, padded, target.file, target.startLine)) { + return { recorded: true, note: 'already recorded for this phase (idempotent)' }; + } + + const key = candidateKey(target.file, target.name); + const description = `${key} — complexity ${target.score}` + + (target.baseline !== null ? ` (baseline ${target.baseline}, delta ${target.delta})` : ''); + + const now = new Date().toISOString(); + try { + const result = windows.appendWindow( + ledger, + { kind: 'deviation', phase: padded, file: target.file, line: target.startLine, description }, + { now }, + ); + writeTextAtomic(windowsLedgerPath(cwd, windows), windows.renderLedger(result.ledger)); + return { recorded: true }; + } catch (e) { + return { recorded: false, note: `failed to record broken-windows entry: ${e instanceof Error ? e.message : String(e)}` }; + } +} + +/** + * Resolve (mark fixed/waived) the ledger window matching phase + target + * file/line, if any. Never throws. Mirrors `recordStrictWindow`'s + * `{ recorded, note }` degrade shape (#1953 defect 2): every + * `resolved: false` path carries a `note` naming the real reason, so + * `accept`/`decline` never tell a user "it failed" without saying why. + */ +function resolveLedgerWindow( + cwd: string, + padded: string, + file: string, + line: number, + kind: 'accept' | 'decline', + reasonText: string, + windowsOverride: WindowsModule | undefined, +): { resolved: boolean; note?: string } { + const loaded = loadWindowsOrDegrade(cwd, windowsOverride); + if (!loaded.ok) return { resolved: false, note: loaded.note }; + const { windows, ledger } = loaded; + + const entry = findOpenDeviationEntry(ledger, padded, file, line); + if (!entry) { + return { resolved: false, note: 'no open broken-windows entry found for this phase/target — nothing to resolve' }; + } + const now = new Date().toISOString(); + try { + const updated = kind === 'accept' + ? windows.markFixed(ledger, entry.id, { now }) + : windows.markWaived(ledger, entry.id, reasonText || 'declined', { now }); + writeTextAtomic(windowsLedgerPath(cwd, windows), windows.renderLedger(updated)); + return { resolved: true }; + } catch (e) { + return { resolved: false, note: `failed to resolve broken-windows entry: ${e instanceof Error ? e.message : String(e)}` }; + } +} + +// ─── evaluate ──────────────────────────────────────────────────────────────── + +const EVALUATE_USAGE = 'Usage: gsd-tools refactor evaluate --phase [--since ] [--raw]'; + +function handleEvaluate( + args: string[], + cwd: string, + raw: boolean, + c: CoreModule, + complexity: ComplexityModule, + git: GitModule, + windowsOverride: WindowsModule | undefined, +): unknown { + const rest = args.slice(2); + const phaseCheck = requirePhaseArg(rest, complexity, EVALUATE_USAGE); + if (!phaseCheck.ok) return phaseCheck.result; + + // Step 3: resolve PHASE_DIR + anchor (phaseStartCommit, or --since override). + const resolved = resolvePhaseDirForArg(cwd, phaseCheck.phase); + if (resolved === null) { + c.output({ verdict: complexity.VERDICT.SKIPPED, reason: complexity.REASON.REFACTOR_INVALID_PHASE, phase: phaseCheck.phase }, raw); + return undefined; + } + const { phaseDir, padded } = resolved; + + const sinceFlag = readFlag(rest, '--since'); + const sinceOverride = sinceFlag.present && !sinceFlag.flagShaped ? sinceFlag.value.trim() : ''; + const sinceRef = sinceOverride !== '' ? sinceOverride : git.phaseStartCommit(cwd, phaseDir); + if (sinceRef === null) { + c.output({ verdict: complexity.VERDICT.SKIPPED, reason: complexity.REASON.REFACTOR_GIT_UNAVAILABLE, phase: padded }, raw); + return undefined; + } + + const touched = git.changedFilesSince(cwd, sinceRef); + if (touched === null) { + c.output({ verdict: complexity.VERDICT.SKIPPED, reason: complexity.REASON.REFACTOR_GIT_UNAVAILABLE, phase: padded }, raw); + return undefined; + } + + // Step 4: empty touched set. + if (touched.length === 0) { + c.output({ verdict: complexity.VERDICT.BELOW_THRESHOLD, reason: complexity.REASON.REFACTOR_NO_TOUCHED_FILES, phase: padded }, raw); + return undefined; + } + + // Step 5: filter with isAnalyzablePath; read each survivor. A read failure + // (or a path that escapes the project root) skips that one file with a + // reason and the run continues — never aborts the evaluation. + const analyzed: AnalyzedFile[] = []; + // Only files that were SUCCESSFULLY analyzed feed nextBaseline's prune set + // — an unreadable file must never wipe that file's baseline history via + // the "file in analyzedFiles but function missing" prune rule. + const successfullyAnalyzedFiles: string[] = []; + for (const relFile of touched) { + if (!complexity.isAnalyzablePath(relFile)) continue; + const confined = resolveConfinedPath(cwd, relFile); + if (confined === null) { + analyzed.push({ file: relFile, ok: false, reason: complexity.REASON.REFACTOR_FILE_UNREADABLE }); + continue; + } + let source: string; + try { + source = fs.readFileSync(confined, 'utf8'); + } catch { + analyzed.push({ file: relFile, ok: false, reason: complexity.REASON.REFACTOR_FILE_UNREADABLE }); + continue; + } + const result = complexity.analyzeSource(source); + if (!result.ok) { + analyzed.push({ file: relFile, ok: false, reason: result.reason }); + } else { + analyzed.push({ file: relFile, ok: true, method: result.method, functions: result.functions }); + successfullyAnalyzedFiles.push(relFile); + } + } + + // Step 6. + const planningDirPath = planningDir(cwd); + const baselineRead = complexity.readBaseline(planningDirPath); + const evalConfig = readEvalConfig(cwd); + const evaluation: Evaluation = complexity.evaluateCandidates({ + analyzed, + baseline: baselineRead.baseline, + threshold: evalConfig.threshold, + jumpDelta: evalConfig.jumpDelta, + }); + + // Step 7. + let artifactWritten = false; + let artifactPath: string | null = null; + let artifactWriteError: string | null = null; + if (evaluation.verdict === complexity.VERDICT.TRIGGERED && evaluation.target) { + const target = evaluation.target; + const proposal: Proposal = { + schema_version: complexity.SCHEMA_VERSION, + status: 'proposed', + phase: padded, + target_file: target.file, + target_function: target.name, + score: target.score, + baseline: target.baseline, + delta: target.delta, + metric: 'decision-points', + recorded_at: new Date().toISOString(), + resolved_at: null, + reason: target.reasons.join(','), + candidates: evaluation.candidates, + }; + artifactPath = artifactAbsPath(cwd, phaseDir, padded, complexity); + try { + writeTextAtomic(artifactPath, complexity.renderProposal(proposal)); + artifactWritten = true; + } catch (e) { + artifactWriteError = e instanceof Error ? e.message : String(e); + } + } + + // Step 8: baseline write failure is reported but does not fail the command. + const nextBaselineValue = complexity.nextBaseline( + baselineRead.baseline, + analyzed, + { analyzedFiles: successfullyAnalyzedFiles, phase: padded }, + ); + const baselineWrite = complexity.writeBaseline(planningDirPath, nextBaselineValue); + + // Step 9: strict mode, TRIGGERED only. + let ledgerRecorded: boolean | undefined; + let ledgerNote: string | undefined; + const warnings: Array<{ reason: string; message: string }> = []; + if (evalConfig.strict && evaluation.verdict === complexity.VERDICT.TRIGGERED && evaluation.target) { + const strictResult = recordStrictWindow(cwd, padded, evaluation.target, windowsOverride); + ledgerRecorded = strictResult.recorded; + ledgerNote = strictResult.note; + + const warning = strictNotEnforcingWarning(complexity, ledgerRecorded, evalConfig.windowsEnforce); + if (warning) warnings.push(warning); + } + + const result: Record = { + verdict: evaluation.verdict, + phase: padded, + candidates: evaluation.candidates, + target: evaluation.target, + skipped: evaluation.skipped, + threshold_used: evaluation.thresholdUsed, + jump_delta_used: evaluation.jumpDeltaUsed, + artifact_written: artifactWritten, + artifact_path: artifactPath, + baseline_write: baselineWrite.ok, + }; + if (artifactWriteError !== null) result.artifact_write_error = artifactWriteError; + if (!baselineWrite.ok && baselineWrite.reason) result.baseline_write_reason = baselineWrite.reason; + if (ledgerRecorded !== undefined) result.ledger_recorded = ledgerRecorded; + if (ledgerNote !== undefined) result.ledger_note = ledgerNote; + if (warnings.length > 0) result.warnings = warnings; + + c.output(result, raw); + return undefined; +} + +// ─── status ────────────────────────────────────────────────────────────────── + +const STATUS_USAGE = 'Usage: gsd-tools refactor status [--phase ] [--raw]'; + +function scanProposals( + cwd: string, + complexity: ComplexityModule, +): Array<{ phase: string; target_file: string; target_function: string; status: string; score: number }> { + const phasesDir = path.join(planningDir(cwd), 'phases'); + const results: Array<{ phase: string; target_file: string; target_function: string; status: string; score: number }> = []; + // #1953 (phase-enumeration-drift guard): phase-directory enumeration has one + // owner (`listMilestonePhaseDirs`, src/phase-locator.cts). Called unscoped + // (no `cwd` in opts) to preserve this scan's prior all-phases-directory + // reach — the only behavior change is that sentinel directories (backlog + // `0-*` / icebox `999-*`, per the canonical `isSentinelPhaseId`) are now + // excluded, and the enumeration order is `comparePhaseNum`-sorted rather + // than raw filesystem order. + const { value: dirs } = listMilestonePhaseDirs(phasesDir); + for (const dirName of dirs) { + const full = path.join(phasesDir, dirName); + let files: string[]; + try { + files = fs.readdirSync(full); + } catch { + continue; + } + for (const fileName of files) { + if (!fileName.endsWith(complexity.PROPOSAL_SUFFIX)) continue; + try { + const text = fs.readFileSync(path.join(full, fileName), 'utf8'); + const proposal = complexity.parseProposal(text); + if (proposal) { + results.push({ + phase: proposal.phase, + target_file: proposal.target_file, + target_function: proposal.target_function, + status: proposal.status, + score: proposal.score, + }); + } + } catch { + continue; + } + } + } + return results; +} + +function handleStatus(args: string[], cwd: string, raw: boolean, c: CoreModule, complexity: ComplexityModule): unknown { + const rest = args.slice(2); + const phaseFlag = readFlag(rest, '--phase'); + if (!phaseFlag.present) { + c.output({ proposals: scanProposals(cwd, complexity) }, raw); + return undefined; + } + + const trimmed = phaseFlag.value.trim(); + if (phaseFlag.duplicated || phaseFlag.flagShaped || trimmed === '' || !PHASE_VALUE_RE.test(trimmed)) { + return makeInvalidArgs( + 'phase', + `Invalid --phase value: ${JSON.stringify(phaseFlag.value)}. ${STATUS_USAGE}`, + complexity.REASON.REFACTOR_INVALID_PHASE, + ); + } + + const resolved = resolvePhaseDirForArg(cwd, trimmed); + if (resolved === null) { + c.output({ found: false, phase: trimmed }, raw); + return undefined; + } + const artifactPath = artifactAbsPath(cwd, resolved.phaseDir, resolved.padded, complexity); + let text: string; + try { + text = fs.readFileSync(artifactPath, 'utf8'); + } catch { + c.output({ found: false, phase: resolved.padded }, raw); + return undefined; + } + const proposal = complexity.parseProposal(text); + if (proposal === null) { + c.output({ found: false, phase: resolved.padded }, raw); + return undefined; + } + c.output({ found: true, phase: resolved.padded, proposal }, raw); + return undefined; +} + +// ─── accept / decline ────────────────────────────────────────────────────── + +/** + * Re-measure the target function's CURRENT complexity score by re-reading + * and re-analyzing `proposal.target_file` — `accept` re-anchors to the + * post-refactor score and `decline` to the current (unchanged) score, and + * both need the LIVE value, not the frozen `proposal.score` snapshot from + * evaluate-time. Falls back to `proposal.score` when the file is gone, + * unreadable, unparseable, or the function can no longer be found (e.g. + * renamed) — reanchorBaseline always needs *a* number, and the proposal's + * last-known score is the least-surprising fallback. + */ +function measureCurrentScore(cwd: string, proposal: Proposal, complexity: ComplexityModule): number { + const confined = resolveConfinedPath(cwd, proposal.target_file); + if (confined === null) return proposal.score; + let source: string; + try { + source = fs.readFileSync(confined, 'utf8'); + } catch { + return proposal.score; + } + const result = complexity.analyzeSource(source); + if (!result.ok) return proposal.score; + const fn = result.functions.find((f) => f.name === proposal.target_function); + return fn ? fn.score : proposal.score; +} + +function dispositionUsage(kind: 'accept' | 'decline'): string { + return kind === 'accept' + ? 'Usage: gsd-tools refactor accept --phase [--raw]' + : 'Usage: gsd-tools refactor decline --phase --reason "" [--raw]'; +} + +function handleDisposition( + kind: 'accept' | 'decline', + args: string[], + cwd: string, + raw: boolean, + c: CoreModule, + complexity: ComplexityModule, + windowsOverride: WindowsModule | undefined, +): unknown { + const rest = args.slice(2); + const usage = dispositionUsage(kind); + + const phaseCheck = requirePhaseArg(rest, complexity, usage); + if (!phaseCheck.ok) return phaseCheck.result; + + let reasonText = ''; + if (kind === 'decline') { + const reasonFlag = readFlag(rest, '--reason'); + const trimmedReason = reasonFlag.value.trim(); + if (!reasonFlag.present || reasonFlag.flagShaped || trimmedReason === '') { + return makeInvalidArgs('reason', usage, complexity.REASON.REFACTOR_DECLINE_REASON_EMPTY); + } + reasonText = reasonFlag.value; + } + + const resolved = resolvePhaseDirForArg(cwd, phaseCheck.phase); + if (resolved === null) { + return makeInvalidArgs( + 'phase', + `No refactor proposal found for phase ${phaseCheck.phase}.`, + complexity.REASON.REFACTOR_ARTIFACT_NOT_FOUND, + ); + } + const artifactPath = artifactAbsPath(cwd, resolved.phaseDir, resolved.padded, complexity); + let text: string; + try { + text = fs.readFileSync(artifactPath, 'utf8'); + } catch { + return makeInvalidArgs( + 'phase', + `No refactor proposal found for phase ${resolved.padded}.`, + complexity.REASON.REFACTOR_ARTIFACT_NOT_FOUND, + ); + } + const proposal = complexity.parseProposal(text); + if (proposal === null) { + return makeInvalidArgs( + 'phase', + `Refactor proposal for phase ${resolved.padded} is unreadable.`, + complexity.REASON.REFACTOR_ARTIFACT_NOT_FOUND, + ); + } + if (proposal.status !== 'proposed') { + return makeInvalidArgs( + 'phase', + `Refactor proposal for phase ${resolved.padded} is already ${proposal.status}.`, + complexity.REASON.REFACTOR_ALREADY_DISPOSITIONED, + ); + } + + const key = candidateKey(proposal.target_file, proposal.target_function); + const now = new Date().toISOString(); + const liveScore = measureCurrentScore(cwd, proposal, complexity); + const targetCandidate = proposal.candidates.find( + (cand) => cand.file === proposal.target_file && cand.name === proposal.target_function, + ) ?? proposal.candidates[0] ?? null; + + const updatedProposal: Proposal = { + ...proposal, + status: kind === 'accept' ? 'accepted' : 'declined', + resolved_at: now, + reason: kind === 'accept' ? proposal.reason : reasonText, + }; + writeTextAtomic(artifactPath, complexity.renderProposal(updatedProposal)); + + const planningDirPath = planningDir(cwd); + const baselineRead = complexity.readBaseline(planningDirPath); + const nextBaselineValue = complexity.reanchorBaseline(baselineRead.baseline, key, liveScore, { phase: resolved.padded }); + const baselineWrite = complexity.writeBaseline(planningDirPath, nextBaselineValue); + + const ledgerResult = resolveLedgerWindow( + cwd, + resolved.padded, + proposal.target_file, + targetCandidate ? targetCandidate.startLine : -1, + kind, + reasonText, + windowsOverride, + ); + + const dispositionResult: Record = { + status: updatedProposal.status, + phase: resolved.padded, + target: key, + reanchored_to: liveScore, + baseline_write: baselineWrite.ok, + ledger_resolved: ledgerResult.resolved, + }; + if (ledgerResult.note !== undefined) dispositionResult.ledger_note = ledgerResult.note; + + c.output(dispositionResult, raw); + return undefined; +} + +// ─── Dispatch ──────────────────────────────────────────────────────────────── + +/** + * Shared capability-active gate + lazy `complexity-trigger.cjs` require + * (#1953 defect 5b) — identical across all four subcommand handlers. Emits + * the disabled response and returns `undefined` without invoking `run` when + * the capability is off; otherwise resolves the module (respecting the + * `_complexity` test seam) and hands it to `run`. + */ +function withActiveComplexity( + cwd: string, + raw: boolean, + c: CoreModule, + complexityOverride: ComplexityModule | undefined, + run: (complexity: ComplexityModule) => unknown, +): unknown { + if (!isCapabilityActive(CAPABILITY_ID, cwd)) { + c.output(disabledResponse(), raw); + return undefined; + } + // eslint-disable-next-line @typescript-eslint/no-require-imports + const complexity: ComplexityModule = complexityOverride ?? (require('./complexity-trigger.cjs') as ComplexityModule); + return run(complexity); +} + +function routeRefactorTriggerCommand({ args, cwd, raw, error, _complexity, _git, _windows, _core }: RouteRefactorTriggerCommandOptions): void { + const c: CoreModule = _core ?? _defaultCore; + + routeHubCommandFamily({ + family: 'refactor', + args, + // Alphabetical for a stable unknown-subcommand message, matching intel/graphify. + subcommands: ['accept', 'decline', 'evaluate', 'status'], + handlers: { + evaluate: () => withActiveComplexity(cwd, raw, c, _complexity, (complexity) => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const git: GitModule = _git ?? (require('./git-base-branch.cjs') as GitModule); + return handleEvaluate(args, cwd, raw, c, complexity, git, _windows); + }), + status: () => withActiveComplexity(cwd, raw, c, _complexity, (complexity) => handleStatus(args, cwd, raw, c, complexity)), + accept: () => withActiveComplexity( + cwd, raw, c, _complexity, + (complexity) => handleDisposition('accept', args, cwd, raw, c, complexity, _windows), + ), + decline: () => withActiveComplexity( + cwd, raw, c, _complexity, + (complexity) => handleDisposition('decline', args, cwd, raw, c, complexity, _windows), + ), + }, + unknownMessage: (subcommand: string, available: string[]) => + `Unknown refactor subcommand. Available: ${available.join(', ')}`, + error, + cwd, + raw, + }); +} + +export = { + routeRefactorTriggerCommand, +}; diff --git a/stryker.config.mjs b/stryker.config.mjs index 6a1255aad..ce4afe8fd 100644 --- a/stryker.config.mjs +++ b/stryker.config.mjs @@ -62,7 +62,7 @@ const UNMUTATED = [ // Full test command used by local runs and as the fallback when CI does not // inject a per-shard command via MUTATION_TEST_CMD. // Keep this list in sync with the tests arrays in scripts/mutation-matrix.cjs COVERED. -const DEFAULT_TEST_CMD = 'node --test tests/context-utilization.property.test.cjs tests/prompt-budget.property.test.cjs tests/frontmatter.property.test.cjs tests/adr-parser.property.test.cjs tests/config-schema.property.test.cjs tests/adr-parser.test.cjs tests/active-workstream-store.test.cjs tests/active-workstream-store.unit.test.cjs tests/prompt-budget.unit.test.cjs tests/adr-parser.unit.test.cjs tests/frontmatter.unit.test.cjs tests/unusable-input.test.cjs tests/core-utils.test.cjs tests/broken-windows.test.cjs'; +const DEFAULT_TEST_CMD = 'node --test tests/context-utilization.property.test.cjs tests/prompt-budget.property.test.cjs tests/frontmatter.property.test.cjs tests/adr-parser.property.test.cjs tests/config-schema.property.test.cjs tests/adr-parser.test.cjs tests/active-workstream-store.test.cjs tests/active-workstream-store.unit.test.cjs tests/prompt-budget.unit.test.cjs tests/adr-parser.unit.test.cjs tests/frontmatter.unit.test.cjs tests/unusable-input.test.cjs tests/core-utils.test.cjs tests/broken-windows.test.cjs tests/complexity-trigger.test.cjs'; /** @type {import('@stryker-mutator/core').PartialStrykerOptions} */ export default { diff --git a/tests/complexity-trigger.test.cjs b/tests/complexity-trigger.test.cjs new file mode 100644 index 000000000..7d8760c47 --- /dev/null +++ b/tests/complexity-trigger.test.cjs @@ -0,0 +1,960 @@ +'use strict'; + +/** + * Complexity-triggered refactor extension point — analyzer, evaluator, and + * baseline-persistence behavioral + property tests. + * + * Module: gsd-core/bin/lib/complexity-trigger.cjs (compiled from + * src/complexity-trigger.cts — deliberately absent; this file is + * written failing-first per issue #1953's TDD directive). + * + * Spec sources (authoritative, do not drift from these without a design + * change): + * - .gsd/phase/feat-1953-complexity-triggered-refactor/41-api-contract.md + * - .gsd/phase/feat-1953-complexity-triggered-refactor/50-test-matrix.md + * - .gsd/phase/feat-1953-complexity-triggered-refactor/40-design.md + * + * Coverage map — test matrix rows 1-54 (analyzer counting, the literal-strip + * leak surface, CRLF/empty/size bounds, properties, file selection, + * threshold/jump evaluation, baseline persistence) plus the typed-surface + * enum/constant lock tests. Rows 55+ (git adapter, CLI, ship gate, loop + * wiring) live in tests/refactor-trigger-cli.test.cjs — out of scope here. + * + * Hermetic: every temp dir via createTempDir + t.after(cleanup); every fs + * mock via mock.method(fs, ...) restored via t.after(...mock.restore()). + * No shared module-level fixture state (matrix row 91). Ambient GSD_* env + * is not touched by this file — every fixture here is either pure in-memory + * input or an isolated tmpdir (matrix row 92 is structural, not exercised + * by this leaf-module suite). + */ + +const { describe, test, mock } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const { createTempDir, cleanup } = require('./helpers.cjs'); +const fc = require('./helpers/fast-check-setup.cjs'); + +const { + BASELINE_FILE_NAME, + PROPOSAL_SUFFIX, + SCHEMA_VERSION, + DEFAULTS, + ANALYZABLE_EXTENSIONS, + VERDICT, + REASON, + stripLiterals, + analyzeSource, + isAnalyzablePath, + evaluateCandidates, + nextBaseline, + reanchorBaseline, + readBaseline, + writeBaseline, +} = require('../gsd-core/bin/lib/complexity-trigger.cjs'); + +// --------------------------------------------------------------------------- +// Analyzer — counting (matrix rows 1-10) +// --------------------------------------------------------------------------- + +describe('complexity-trigger: analyzer — counting decision points', () => { + test('scoresFlatFunctionAsOne', () => { + const source = ['function f() {', ' return 1;', '}'].join('\n'); + const r = analyzeSource(source); + assert.equal(r.ok, true); + assert.equal(r.functions.length, 1); + assert.equal(r.functions[0].name, 'f'); + assert.equal(r.functions[0].startLine, 1); + assert.equal(r.functions[0].score, 1); + }); + + test('scoresSingleIfAsTwo', () => { + const source = [ + 'function f(x) {', + ' if (x) {', + ' return 1;', + ' }', + '}', + ].join('\n'); + const r = analyzeSource(source); + assert.equal(r.ok, true); + assert.equal(r.functions[0].score, 2); + }); + + test('countsEachDecisionConstructOnce', () => { + // Each of the twelve listed constructs exactly once: if, else if, for, + // for..of, for..in, do, while (via one do-while so "while" is not + // double-counted by a separate stand-alone while statement), case, + // catch, &&, ||, ?:. Base 1 + 12 = 13. + const source = [ + 'function f(x) {', + ' if (x === 1) { return 1; }', + ' else if (x === 2) { return 2; }', + ' for (let i = 0; i < 1; i++) { g(i); }', + ' for (const y of x) { g(y); }', + ' for (const k in x) { g(k); }', + ' let n = 0;', + ' do { n += 1; } while (n < 1);', + ' switch (x) {', + ' case 1: g(1); break;', + ' default: g(0);', + ' }', + ' try { g(x); } catch (e) { g(e); }', + ' const a = x && 1;', + ' const b = x || 1;', + ' const c = x ? 1 : 2;', + ' return a + b + c;', + '}', + ].join('\n'); + const r = analyzeSource(source); + assert.equal(r.ok, true); + assert.equal(r.functions.length, 1); + assert.equal(r.functions[0].score, 13); + }); + + test('doesNotCountBareElse', () => { + const source = [ + 'function f(x) {', + ' if (x) { return 1; }', + ' else { return 2; }', + '}', + ].join('\n'); + const r = analyzeSource(source); + assert.equal(r.functions[0].score, 2, 'bare else must not add a point beyond the if'); + }); + + test('doesNotCountSwitchDefault', () => { + const source = [ + 'function f(x) {', + ' switch (x) {', + ' case 1: return 1;', + ' default: return 0;', + ' }', + '}', + ].join('\n'); + const r = analyzeSource(source); + assert.equal(r.functions[0].score, 2, 'base 1 + one case; default: must not add a point'); + }); + + test('doesNotConflateLengthWithComplexity', () => { + const bodyLines = Array.from({ length: 200 }, (_, i) => ` const v${i} = ${i};`); + const source = ['function f() {', ...bodyLines, ' return 0;', '}'].join('\n'); + const r = analyzeSource(source); + assert.equal(r.ok, true); + assert.equal(r.functions[0].score, 1, 'a long but flat function must still score 1'); + }); + + test('scoresFlatSwitchByCaseCountPinningKnownBias', () => { + const caseLines = Array.from({ length: 12 }, (_, i) => ` case ${i + 1}: return ${i + 1};`); + const source = [ + 'function f(x) {', + ' switch (x) {', + ...caseLines, + ' }', + ' return 0;', + '}', + ].join('\n'); + const r = analyzeSource(source); + assert.equal(r.functions[0].score, 13, 'flat 12-case switch scores 1 + 12 by design — the known bias is pinned, not accidental'); + }); + + test('attributesScoresToEachFunction', () => { + const source = [ + 'function a() {', + ' return 1;', + '}', + '', + 'function b(x) {', + ' if (x) {', + ' return 1;', + ' }', + ' return 0;', + '}', + ].join('\n'); + const r = analyzeSource(source); + assert.equal(r.functions.length, 2); + assert.equal(r.functions[0].name, 'a'); + assert.equal(r.functions[0].startLine, 1); + assert.equal(r.functions[0].score, 1); + assert.equal(r.functions[1].name, 'b'); + assert.equal(r.functions[1].startLine, 5); + assert.equal(r.functions[1].score, 2); + }); + + test('doesNotDoubleCountNestedFunction', () => { + const source = [ + 'function outer() {', + ' function inner(x) {', + ' if (x) {', + ' return 1;', + ' }', + ' return 0;', + ' }', + ' return inner;', + '}', + ].join('\n'); + const r = analyzeSource(source); + assert.equal(r.functions.length, 2); + const outer = r.functions.find((fn) => fn.name === 'outer'); + const inner = r.functions.find((fn) => fn.name === 'inner'); + assert.equal(outer.score, 1, 'the nested if must not double-count into outer'); + assert.equal(inner.score, 2, 'the nested if is attributed to the innermost function'); + }); + + test('detectsAllFunctionForms', () => { + const source = [ + 'const arrow = (x) => x + 1;', + 'const obj = {', + ' method(x) {', + ' return x;', + ' },', + '};', + 'async function asyncFn() {', + ' return 1;', + '}', + 'function* genFn() {', + ' yield 1;', + '}', + ].join('\n'); + const r = analyzeSource(source); + assert.equal(r.ok, true); + assert.equal(r.functions.length, 4, 'arrow, method shorthand, async function, and generator must all be detected'); + assert.ok(r.functions.every((fn) => fn.score === 1), 'none of these forms has a branch of its own'); + }); +}); + +// --------------------------------------------------------------------------- +// Analyzer — the leak surface (matrix rows 11-21) +// --------------------------------------------------------------------------- + +describe('complexity-trigger: analyzer — the leak surface', () => { + test('ignoresDecisionKeywordInLineComment', () => { + const source = [ + 'function f() {', + ' // if (x) { return 1; }', + ' return 0;', + '}', + ].join('\n'); + assert.equal(analyzeSource(source).functions[0].score, 1); + }); + + test('ignoresDecisionKeywordInBlockComment', () => { + const source = [ + 'function f() {', + ' /* if (x) { return 1 } && y */', + ' return 0;', + '}', + ].join('\n'); + assert.equal(analyzeSource(source).functions[0].score, 1); + }); + + test('ignoresDecisionKeywordInString', () => { + const source = [ + 'function f() {', + ' const s = "if (x) { return 1; }";', + ' return s;', + '}', + ].join('\n'); + assert.equal(analyzeSource(source).functions[0].score, 1); + }); + + test('ignoresDecisionKeywordInTemplateLiteral', () => { + const source = [ + 'function f() {', + ' const s = `if (x) { return 1; }`;', + ' return s;', + '}', + ].join('\n'); + assert.equal(analyzeSource(source).functions[0].score, 1); + }); + + test('ignoresAlternationInsideRegexLiteral', () => { + const source = [ + 'function f(s) {', + ' return /a|b/.test(s);', + '}', + ].join('\n'); + assert.equal(analyzeSource(source).functions[0].score, 1, 'the | inside /a|b/ must not count as ||'); + }); + + test('doesNotMistakeDivisionForRegexLiteral', () => { + const source = [ + 'function f(a, b, c) {', + ' return a / b / c;', + '}', + ].join('\n'); + const r = analyzeSource(source); + assert.equal(r.ok, true); + assert.equal(r.functions[0].score, 1, 'a / b / c must be read as division, never as a regex literal'); + }); + + test('doesNotCountOptionalChainingAsTernary', () => { + const source = [ + 'function f(a) {', + ' return a?.b;', + '}', + ].join('\n'); + assert.equal(analyzeSource(source).functions[0].score, 1); + }); + + test('doesNotCountNullishCoalescingAsBranch', () => { + const source = [ + 'function f(x, y) {', + ' return x ?? y;', + '}', + ].join('\n'); + assert.equal(analyzeSource(source).functions[0].score, 1); + }); + + test('handlesEscapedQuoteWithoutSwallowingCode', () => { + const source = [ + 'function f(x) {', + " const s = 'it\\'s fine';", + ' if (x) { return s; }', + ' return null;', + '}', + ].join('\n'); + const r = analyzeSource(source); + assert.equal(r.ok, true); + assert.equal(r.functions[0].score, 2, 'the escaped quote must not swallow the following if'); + }); + + test('countsBranchInsideTemplateInterpolation', () => { + const source = [ + 'function f(x) {', + ' const s = `value: ${x ? 1 : 2}`;', + ' return s;', + '}', + ].join('\n'); + const r = analyzeSource(source); + assert.equal(r.ok, true); + assert.equal(r.functions[0].score, 2, 'the ternary inside ${} is real code and must be counted'); + }); + + test('refusesToScoreUnterminatedLiteral', () => { + const unterminatedString = [ + 'function f() {', + ' const s = "never closed;', + '}', + ].join('\n'); + const unterminatedTemplate = [ + 'function f() {', + ' const s = `never closed;', + '}', + ].join('\n'); + const unterminatedComment = [ + 'function f() {', + ' /* never closed', + ' return 1;', + '}', + ].join('\n'); + + for (const src of [unterminatedString, unterminatedTemplate, unterminatedComment]) { + const r = analyzeSource(src); + assert.equal(r.ok, false); + assert.equal(r.reason, REASON.REFACTOR_ANALYZER_UNPARSEABLE); + assert.equal(r.functions, undefined, 'an unparseable source must never emit a number'); + } + }); + + test('producesIdenticalScoresForCrlfAndLf', () => { + const lfSource = [ + 'function f(x) {', + ' if (x) {', + ' return 1;', + ' } else if (x === 2) {', + ' return 2;', + ' }', + ' return 0;', + '}', + '', + 'function g(y) {', + ' return y && y || y ? 1 : 0;', + '}', + ].join('\n'); + const crlfSource = lfSource.replace(/\n/g, '\r\n'); + + const lfResult = analyzeSource(lfSource); + const crlfResult = analyzeSource(crlfSource); + assert.equal(lfResult.ok, true); + assert.equal(crlfResult.ok, true); + assert.deepEqual( + crlfResult.functions, + lfResult.functions, + 'CRLF and LF forms of the same source must yield byte-identical scores AND line numbers', + ); + }); + + test('returnsEmptyResultForEmptyFile', () => { + const r = analyzeSource(''); + assert.equal(r.ok, true); + assert.deepEqual(r.functions, []); + }); + + test('returnsEmptyResultForWhitespaceOnlyFile', () => { + const r = analyzeSource(' \n\t\n \n'); + assert.equal(r.ok, true); + assert.deepEqual(r.functions, []); + }); + + test('returnsEmptyResultWhenNoFunctionsPresent', () => { + const source = [ + 'const x = 1;', + 'if (x) {', + ' console.log(x);', + '}', + ].join('\n'); + const r = analyzeSource(source); + assert.equal(r.ok, true); + assert.deepEqual(r.functions, [], 'top-level code belongs to no function, and must never be a synthetic entry'); + }); + + test('analyzesLargeFileWithinBounds', () => { + const N = 20000; + const chunks = Array.from({ length: N }, (_, i) => [ + `function f${i}(x) {`, + ' if (x) { return 1; }', + ' return 0;', + '}', + ].join('\n')); + const source = chunks.join('\n\n'); + assert.ok(source.length > 1_000_000, 'fixture must actually be large enough to exercise the bound'); + + const r = analyzeSource(source); + assert.equal(r.ok, true); + assert.equal(r.functions.length, N); + assert.ok(r.functions.every((fn) => fn.score === 2)); + }); +}); + +// --------------------------------------------------------------------------- +// Analyzer — properties (matrix rows 27-28) +// --------------------------------------------------------------------------- + +describe('complexity-trigger: analyzer — properties', () => { + test('propertyStripNeverManufacturesDecisionPoints', () => { + // #1953 defect 3: the invariant is asserted through the TYPED surface + // (analyzeSource's numeric score), never by pattern-matching the text + // stripLiterals produced (CONTRIBUTING.md:800 bans grepping the SUT's + // own output, with no carve-out for a text transform's own return + // value). A fuzzed string embedded as INERT content (a block comment, + // or a string literal) can never manufacture a decision point — proven + // by the reported score staying byte-for-byte the same number. + const BASE = ['function f(x) {', ' if (x) { return 1; }', ' return 0;', '}', ''].join('\n'); + const baseAnalyzed = analyzeSource(BASE); + assert.equal(baseAnalyzed.ok, true); + const baselineScores = baseAnalyzed.functions.map((fn) => fn.score); + + fc.assert( + fc.property(fc.string({ maxLength: 300 }), (source) => { + const stripped = stripLiterals(source); + if (stripped.ok) { + assert.ok( + stripped.stripped.length <= source.length, + `stripLiterals must never grow the source (in=${source.length}, out=${stripped.stripped.length})`, + ); + } + const analyzed = analyzeSource(source); + if (analyzed.ok) { + for (const fn of analyzed.functions) { + assert.ok(fn.score >= 1, 'every detected function must score at least 1'); + } + } + + // Sanitize just enough to keep the wrapper well-formed (never + // "*/" inside the comment; never an unescaped quote, backslash, or + // raw newline inside the string) — the content is otherwise + // untouched, including any decision-point-shaped substrings. + const commentSafe = source.replace(/\*/g, ' '); + const withComment = analyzeSource(`${BASE}\n/* ${commentSafe} */\n`); + assert.equal(withComment.ok, true, 'a sanitized block comment must never make the source unparseable'); + assert.deepEqual( + withComment.functions.map((fn) => fn.score), + baselineScores, + 'embedding fuzzed text inside a block comment must never change the reported score', + ); + + const stringSafe = source.replace(/["\\\r\n]/g, ' '); + const withString = analyzeSource(`${BASE}\nconst __s = "${stringSafe}";\n`); + assert.equal(withString.ok, true, 'a sanitized string literal must never make the source unparseable'); + assert.deepEqual( + withString.functions.map((fn) => fn.score), + baselineScores, + 'embedding fuzzed text inside a string literal must never change the reported score', + ); + }), + { numRuns: 50, seed: 42 }, + ); + }); + + test('propertyCommentsAndStringsAreScoreNeutral', () => { + const BASE_SOURCES = [ + ['function f(x) {', ' if (x) { return 1; }', ' return 0;', '}'].join('\n'), + ['function g(x, y) {', ' return x && y;', '}'].join('\n'), + ['function h(x) {', ' for (let i = 0; i < x; i++) { g(i); }', '}'].join('\n'), + ]; + const APPENDERS = [ + (src) => [src, '// if (x) { return 1; } && y || z ? 1 : 2', ''].join('\n'), + (src) => [src, '/* if (x) { return 1 } && y */', ''].join('\n'), + (src) => [src, 'const __s = "if (x) { return 1; } && y || z ? 1 : 2";', ''].join('\n'), + ]; + fc.assert( + fc.property( + fc.constantFrom(...BASE_SOURCES), + fc.constantFrom(...APPENDERS), + (base, appendFn) => { + const before = analyzeSource(base); + const after = analyzeSource(appendFn(base)); + assert.equal(before.ok, true); + assert.equal(after.ok, true); + assert.deepEqual( + before.functions.map((fn) => fn.score), + after.functions.map((fn) => fn.score), + 'appending a comment/string containing decision keywords must never change a score', + ); + }, + ), + { numRuns: 20, seed: 42 }, + ); + }); +}); + +// --------------------------------------------------------------------------- +// File selection (matrix rows 29-32) +// --------------------------------------------------------------------------- + +describe('complexity-trigger: file selection', () => { + test('analyzesSupportedExtensions', () => { + for (const ext of ANALYZABLE_EXTENSIONS) { + assert.equal(isAnalyzablePath(`src/foo${ext}`), true, `expected ${ext} to be analyzable`); + } + }); + + test('skipsUnsupportedExtensionsWithoutScoringZero', () => { + for (const p of ['docs/readme.md', 'package.json', 'ci.yml', 'notes.txt', 'Makefile']) { + assert.equal(isAnalyzablePath(p), false, `expected ${p} to be unsupported`); + } + }); + + test('excludesTestFilesByDefault', () => { + assert.equal(isAnalyzablePath('tests/foo.test.cjs'), false); + assert.equal(isAnalyzablePath('tests\\foo.test.cjs'), false, 'separators are normalized unconditionally'); + }); + + test('excludesGeneratedLibPathsExplicitly', () => { + assert.equal(isAnalyzablePath('gsd-core/bin/lib/foo.cjs'), false); + assert.equal(isAnalyzablePath('gsd-core\\bin\\lib\\foo.cjs'), false); + }); +}); + +// --------------------------------------------------------------------------- +// Threshold / jump evaluation (matrix rows 33-44) +// --------------------------------------------------------------------------- + +describe('complexity-trigger: threshold / jump evaluation', () => { + test('doesNotTriggerBelowThreshold', () => { + const T = DEFAULTS.threshold; + const analyzed = [{ file: 'a.js', ok: true, method: 'decision-points', functions: [{ name: 'f', startLine: 1, endLine: 3, score: T - 1 }] }]; + const r = evaluateCandidates({ analyzed, baseline: {} }); + assert.equal(r.verdict, VERDICT.BELOW_THRESHOLD); + assert.deepEqual(r.candidates, []); + assert.equal(r.target, null); + }); + + test('doesNotTriggerAtExactThreshold', () => { + const T = DEFAULTS.threshold; + const analyzed = [{ file: 'a.js', ok: true, method: 'decision-points', functions: [{ name: 'f', startLine: 1, endLine: 3, score: T }] }]; + const r = evaluateCandidates({ analyzed, baseline: {} }); + assert.equal(r.verdict, VERDICT.BELOW_THRESHOLD, 'strictly-greater semantics: score === T must not trigger'); + assert.deepEqual(r.candidates, []); + }); + + test('triggersAboveThreshold', () => { + const T = DEFAULTS.threshold; + const analyzed = [{ file: 'a.js', ok: true, method: 'decision-points', functions: [{ name: 'f', startLine: 1, endLine: 3, score: T + 1 }] }]; + const r = evaluateCandidates({ analyzed, baseline: {} }); + assert.equal(r.verdict, VERDICT.TRIGGERED); + assert.equal(r.candidates.length, 1); + assert.deepEqual(r.candidates[0].reasons, ['threshold']); + assert.equal(r.candidates[0].baseline, null); + assert.equal(r.candidates[0].delta, null); + }); + + test('doesNotTriggerBelowJumpDelta', () => { + const T = DEFAULTS.threshold; + const D = DEFAULTS.jumpDelta; + const score = T - 5; + const baseline = { 'a.js::f': { score: score - (D - 1) } }; + const analyzed = [{ file: 'a.js', ok: true, method: 'decision-points', functions: [{ name: 'f', startLine: 1, endLine: 3, score }] }]; + const r = evaluateCandidates({ analyzed, baseline }); + assert.equal(r.verdict, VERDICT.BELOW_THRESHOLD); + assert.deepEqual(r.candidates, []); + }); + + test('doesNotTriggerAtExactJumpDelta', () => { + const T = DEFAULTS.threshold; + const D = DEFAULTS.jumpDelta; + const score = T - 5; + const baseline = { 'a.js::f': { score: score - D } }; + const analyzed = [{ file: 'a.js', ok: true, method: 'decision-points', functions: [{ name: 'f', startLine: 1, endLine: 3, score }] }]; + const r = evaluateCandidates({ analyzed, baseline }); + assert.equal(r.verdict, VERDICT.BELOW_THRESHOLD, 'strictly-greater semantics: delta === D must not trigger'); + }); + + test('triggersAboveJumpDelta', () => { + const T = DEFAULTS.threshold; + const D = DEFAULTS.jumpDelta; + const score = T - 5; + const baseline = { 'a.js::f': { score: score - (D + 1) } }; + const analyzed = [{ file: 'a.js', ok: true, method: 'decision-points', functions: [{ name: 'f', startLine: 1, endLine: 3, score }] }]; + const r = evaluateCandidates({ analyzed, baseline }); + assert.equal(r.verdict, VERDICT.TRIGGERED); + assert.deepEqual(r.candidates[0].reasons, ['jump']); + }); + + test('emitsSingleCandidateCarryingBothReasons', () => { + const T = DEFAULTS.threshold; + const D = DEFAULTS.jumpDelta; + const score = T + 1; + const baseline = { 'a.js::f': { score: score - (D + 1) } }; + const analyzed = [{ file: 'a.js', ok: true, method: 'decision-points', functions: [{ name: 'f', startLine: 1, endLine: 3, score }] }]; + const r = evaluateCandidates({ analyzed, baseline }); + assert.equal(r.candidates.length, 1, 'one candidate, never two entries'); + assert.deepEqual(r.candidates[0].reasons, ['threshold', 'jump'], 'both reasons, ordered'); + }); + + test('doesNotTriggerWhenComplexityDecreased', () => { + const analyzed = [{ file: 'a.js', ok: true, method: 'decision-points', functions: [{ name: 'f', startLine: 1, endLine: 3, score: 10 }] }]; + const baseline = { 'a.js::f': { score: 20 } }; + const r = evaluateCandidates({ analyzed, baseline }); + assert.equal(r.verdict, VERDICT.BELOW_THRESHOLD); + assert.deepEqual(r.candidates, []); + }); + + test('doesNotTreatMissingBaselineAsZeroBaseline', () => { + const T = DEFAULTS.threshold; + const analyzed = [{ file: 'a.js', ok: true, method: 'decision-points', functions: [{ name: 'f', startLine: 1, endLine: 3, score: T + 1 }] }]; + const r = evaluateCandidates({ analyzed, baseline: {} }); + assert.equal(r.candidates.length, 1); + assert.equal(r.candidates[0].baseline, null); + assert.equal(r.candidates[0].delta, null, 'no baseline means delta is null — never equal to score'); + assert.notEqual(r.candidates[0].delta, r.candidates[0].score); + assert.deepEqual(r.candidates[0].reasons, ['threshold'], 'jump must not be evaluated with no baseline'); + }); + + test('ordersCandidatesDeterministically', () => { + const analyzed = [ + { file: 'b.js', ok: true, method: 'decision-points', functions: [{ name: 'y', startLine: 1, endLine: 3, score: 20 }] }, + { + file: 'a.js', + ok: true, + method: 'decision-points', + functions: [ + { name: 'x', startLine: 1, endLine: 3, score: 20 }, + { name: 'z', startLine: 10, endLine: 13, score: 18 }, + ], + }, + ]; + const baseline = { 'a.js::x': { score: 10 } }; // y and z have no baseline entry + const r = evaluateCandidates({ analyzed, baseline, threshold: 15, jumpDelta: 100 }); + const ids = r.candidates.map((c) => `${c.file}::${c.name}`); + assert.deepEqual(ids, ['a.js::x', 'b.js::y', 'a.js::z'], 'score desc, delta desc (null last), file asc, name asc'); + assert.equal(r.target, r.candidates[0]); + }); + + test('breaksCandidateTiesStably', () => { + const analyzed = [ + { + file: 'a.js', + ok: true, + method: 'decision-points', + functions: [ + { name: 'bravo', startLine: 20, endLine: 22, score: 20 }, + { name: 'alpha', startLine: 5, endLine: 7, score: 20 }, + ], + }, + ]; + const r = evaluateCandidates({ analyzed, baseline: {}, threshold: 15, jumpDelta: 100 }); + assert.deepEqual(r.candidates.map((c) => c.name), ['alpha', 'bravo'], 'tie on every other key breaks by name ascending'); + }); + + test('rejectsNonNumericThresholdInsteadOfNaNComparing', () => { + const scoreAboveDefaultThreshold = DEFAULTS.threshold + 1; + const analyzed = [{ + file: 'a.js', + ok: true, + method: 'decision-points', + functions: [{ name: 'f', startLine: 1, endLine: 3, score: scoreAboveDefaultThreshold }], + }]; + for (const bad of [0, -5, 'not-a-number', NaN, undefined, null, {}]) { + const r = evaluateCandidates({ analyzed, baseline: {}, threshold: bad, jumpDelta: bad }); + assert.equal(r.thresholdUsed, DEFAULTS.threshold, `threshold ${String(bad)} must fall back to the default`); + assert.equal(r.jumpDeltaUsed, DEFAULTS.jumpDelta, `jumpDelta ${String(bad)} must fall back to the default`); + assert.equal(r.verdict, VERDICT.TRIGGERED, 'fallback must still evaluate — never a silent NaN-comparison never-trigger'); + } + }); +}); + +// --------------------------------------------------------------------------- +// Baseline persistence (matrix rows 45-54) +// --------------------------------------------------------------------------- + +describe('complexity-trigger: baseline persistence', () => { + test('treatsMissingBaselineAsEmpty', (t) => { + const tmp = createTempDir('complexity-trigger-'); + t.after(() => cleanup(tmp)); + const r = readBaseline(tmp); + assert.equal(r.ok, true); + assert.deepEqual(r.baseline, {}); + assert.equal(r.reason, undefined); + }); + + test('doesNotClobberMalformedBaselineOnReadFailure', (t) => { + const tmp = createTempDir('complexity-trigger-'); + t.after(() => cleanup(tmp)); + const baselinePath = path.join(tmp, BASELINE_FILE_NAME); + fs.writeFileSync(baselinePath, '{not valid json', 'utf8'); + const before = fs.readFileSync(baselinePath, 'utf8'); + + const r = readBaseline(tmp); + assert.equal(r.ok, false); + assert.equal(r.reason, REASON.REFACTOR_BASELINE_MALFORMED); + assert.deepEqual(r.baseline, {}); + + const after = fs.readFileSync(baselinePath, 'utf8'); + assert.equal(after, before, 'a failing read must never rewrite the malformed file'); + }); + + test('rejectsWrongShapedBaseline', (t) => { + const tmp = createTempDir('complexity-trigger-'); + t.after(() => cleanup(tmp)); + const baselinePath = path.join(tmp, BASELINE_FILE_NAME); + for (const bad of ['[]', '"a string"', 'null', '42']) { + fs.writeFileSync(baselinePath, bad, 'utf8'); + const r = readBaseline(tmp); + assert.equal(r.ok, false, `shape ${bad} must be rejected`); + assert.equal(r.reason, REASON.REFACTOR_BASELINE_MALFORMED); + } + }); + + test('anchorsBaselineOnFirstObservation', () => { + const prev = {}; + const analyzed = [{ + file: 'a.js', ok: true, method: 'decision-points', + functions: [{ name: 'f', startLine: 1, endLine: 3, score: 6 }], + }]; + const next = nextBaseline(prev, analyzed, { analyzedFiles: ['a.js'] }); + assert.equal(next['a.js::f'].score, 6, 'first observation inserts the anchor at the current score'); + }); + + test('doesNotAdvanceAnchorOnPlainEvaluate', () => { + const prev = { 'a.js::f': { score: 3 } }; + const analyzed = [{ + file: 'a.js', ok: true, method: 'decision-points', + functions: [{ name: 'f', startLine: 1, endLine: 3, score: 6 }], + }]; + const next = nextBaseline(prev, analyzed, { analyzedFiles: ['a.js'] }); + assert.equal(next['a.js::f'].score, 3, 'a stable anchor must not advance on a plain evaluate, even for a non-triggering function'); + }); + + test('freezesBaselineWhileProposalUntriaged', () => { + // The anchor is unconditionally stable across evaluate — not merely + // "frozen because it triggered". This proves it holds even when the + // function is (still) triggering on a second evaluate. + const prev = { 'a.js::f': { score: 10 } }; + const analyzed = [{ + file: 'a.js', ok: true, method: 'decision-points', + functions: [{ name: 'f', startLine: 1, endLine: 5, score: DEFAULTS.threshold + 5 }], + }]; + const evaluation = evaluateCandidates({ analyzed, baseline: prev }); + assert.equal(evaluation.verdict, VERDICT.TRIGGERED); + + const next = nextBaseline(prev, analyzed, { analyzedFiles: ['a.js'] }); + assert.equal(next['a.js::f'], prev['a.js::f'], 'the anchor carries forward unchanged, not merely equal'); + assert.equal(next['a.js::f'].score, 10); + }); + + test('reanchorsBaselineOnDisposition', () => { + // Proves reanchorBaseline is the ONLY thing that moves an anchor: + // nextBaseline leaves it untouched regardless of candidates; only an + // explicit disposition call moves it. + const key = 'src/a.cts::target'; + const prev = { [key]: { score: 3 } }; + + const analyzed = [{ + file: 'src/a.cts', ok: true, method: 'decision-points', + functions: [{ name: 'target', startLine: 1, endLine: 20, score: 11 }], + }]; + const evaluation = evaluateCandidates({ analyzed, baseline: prev }); + assert.equal(evaluation.verdict, VERDICT.TRIGGERED, 'setup sanity: the function does trigger, proving the freeze below is not merely "nothing to move"'); + const afterEvaluate = nextBaseline(prev, analyzed, { analyzedFiles: ['src/a.cts'] }); + assert.equal(afterEvaluate[key].score, 3, 'a plain evaluate must never move the anchor'); + + // Accept: re-anchor to a LOWER post-refactor score. + const afterAccept = reanchorBaseline(prev, key, 4); + assert.equal(afterAccept[key].score, 4, 'accept re-anchors to the post-refactor score'); + + // Decline: re-anchor to the current HIGHER score (debt consciously accepted). + const afterDecline = reanchorBaseline(prev, key, 11); + assert.equal(afterDecline[key].score, 11, 'decline re-anchors to the current score'); + }); + + test('detectsIncrementalCreepAcrossRuns', () => { + const T = 15; + const D = 5; + const anchor = { 'a.js::f': { score: 8 } }; + let baseline = anchor; + let firstTriggeringRound = null; + let firstTriggeringCandidate = null; + + for (let round = 1; round <= 5; round += 1) { + const score = 6 + round * 2; // round1=8, round2=10, round3=12, round4=14, round5=16 + const analyzed = [{ + file: 'a.js', ok: true, method: 'decision-points', + functions: [{ name: 'f', startLine: 1, endLine: 5, score }], + }]; + const evaluation = evaluateCandidates({ analyzed, baseline, threshold: T, jumpDelta: D }); + if (firstTriggeringRound === null && evaluation.verdict === VERDICT.TRIGGERED) { + firstTriggeringRound = round; + firstTriggeringCandidate = evaluation.candidates[0]; + } + baseline = nextBaseline(baseline, analyzed, { analyzedFiles: ['a.js'] }); + } + + // Anchor is stable at 8 throughout (a plain evaluate never advances it), + // so the delta is CUMULATIVE since the anchor: round1=0, round2=2, + // round3=4, round4=6, round5=8. Delta first exceeds D=5 at round 4 + // (6 > 5) — one full run before the absolute score would cross T=15 + // (score 16 at round 5). This is the point of the test: the jump-delta + // catches the creep earlier than, and independently of, the absolute + // threshold — it is not redundant with it. + assert.equal(firstTriggeringRound, 4); + assert.ok( + firstTriggeringCandidate.reasons.includes('jump'), + 'round 4 must trigger via the jump reason, proving the jump-delta adds value over the absolute threshold', + ); + }); + + test('prunesBaselineForDeletedFile', () => { + const analyzed = [{ + file: 'a.js', ok: true, method: 'decision-points', + functions: [{ name: 'f', startLine: 1, endLine: 3, score: 5 }], + }]; + + const prev = { 'a.js::f': { score: 5 }, 'a.js::g': { score: 8 } }; + const next = nextBaseline(prev, analyzed, { analyzedFiles: ['a.js'] }); + assert.ok(!('a.js::g' in next), 'g no longer exists in the analyzed file; its baseline entry must be pruned'); + assert.equal(next['a.js::f'].score, 5); + + const prevWithUntouchedFile = { 'a.js::f': { score: 5 }, 'b.js::h': { score: 99 } }; + const nextWithUntouchedFile = nextBaseline(prevWithUntouchedFile, analyzed, { analyzedFiles: ['a.js'] }); + assert.equal( + nextWithUntouchedFile['b.js::h'].score, + 99, + 'an entry whose file is absent from analyzedFiles must never be pruned', + ); + }); + + test('leavesNoPartialBaselineWhenWriteFails', (t) => { + const tmp = createTempDir('complexity-trigger-'); + t.after(() => cleanup(tmp)); + const writeMock = mock.method(fs, 'writeFileSync', () => { + const err = new Error('ENOSPC: no space left on device'); + err.code = 'ENOSPC'; + throw err; + }); + t.after(() => writeMock.mock.restore()); + + const r = writeBaseline(tmp, { 'a.js::f': { score: 5 } }); + assert.equal(r.ok, false); + assert.equal(r.reason, REASON.REFACTOR_BASELINE_WRITE_FAILED); + + const leftover = fs.readdirSync(tmp).filter((n) => n.includes('.tmp')); + assert.deepEqual(leftover, [], 'no orphan tmp file must remain after write failure'); + }); + + test('cleansUpTempFileWhenRenameFails', (t) => { + const tmp = createTempDir('complexity-trigger-'); + t.after(() => cleanup(tmp)); + const renameMock = mock.method(fs, 'renameSync', () => { + const err = new Error('EPERM: operation not permitted'); + err.code = 'EPERM'; + throw err; + }); + t.after(() => renameMock.mock.restore()); + + const r = writeBaseline(tmp, { 'a.js::f': { score: 5 } }); + assert.equal(r.ok, false); + assert.equal(r.reason, REASON.REFACTOR_BASELINE_WRITE_FAILED); + + const leftover = fs.readdirSync(tmp).filter((n) => n.includes('.tmp')); + assert.deepEqual(leftover, [], 'the orphan .tmp file must be unlinked when rename fails'); + }); + + test('degradesReadBaselineOnDeniedRead', (t) => { + // An injected EACCES on the baseline read must degrade readBaseline to + // { ok:false, baseline:{}, reason: REFACTOR_BASELINE_MALFORMED } without + // throwing. + const tmp = createTempDir('complexity-trigger-'); + t.after(() => cleanup(tmp)); + const readMock = mock.method(fs, 'readFileSync', () => { + const err = new Error('EACCES: permission denied'); + err.code = 'EACCES'; + throw err; + }); + t.after(() => readMock.mock.restore()); + + const r = readBaseline(tmp); + assert.equal(r.ok, false); + assert.deepEqual(r.baseline, {}); + assert.equal(r.reason, REASON.REFACTOR_BASELINE_MALFORMED); + }); +}); + +// --------------------------------------------------------------------------- +// Typed surface — frozen enums and constants +// --------------------------------------------------------------------------- + +describe('complexity-trigger: typed surface', () => { + test('locksReasonEnumKeys', () => { + const expected = [ + 'REFACTOR_ALREADY_DISPOSITIONED', + 'REFACTOR_ANALYZER_UNPARSEABLE', + 'REFACTOR_ANALYZER_UNSUPPORTED', + 'REFACTOR_ARTIFACT_NOT_FOUND', + 'REFACTOR_BASELINE_MALFORMED', + 'REFACTOR_BASELINE_WRITE_FAILED', + 'REFACTOR_DECLINE_REASON_EMPTY', + 'REFACTOR_DISABLED', + 'REFACTOR_FILE_UNREADABLE', + 'REFACTOR_GIT_UNAVAILABLE', + 'REFACTOR_INVALID_PHASE', + 'REFACTOR_NO_TOUCHED_FILES', + 'REFACTOR_OK', + 'REFACTOR_STRICT_NOT_ENFORCING', + 'REFACTOR_USAGE', + ].sort(); + assert.deepEqual(Object.keys(REASON).sort(), expected); + assert.ok(Object.isFrozen(REASON)); + }); + + test('locksVerdictEnumKeys', () => { + const expected = ['BELOW_THRESHOLD', 'SKIPPED', 'TRIGGERED'].sort(); + assert.deepEqual(Object.keys(VERDICT).sort(), expected); + assert.ok(Object.isFrozen(VERDICT)); + }); + + test('exposes frozen DEFAULTS and ANALYZABLE_EXTENSIONS with documented values', () => { + assert.deepEqual(DEFAULTS, { threshold: 15, jumpDelta: 5 }); + assert.ok(Object.isFrozen(DEFAULTS)); + assert.deepEqual(ANALYZABLE_EXTENSIONS, ['.js', '.cjs', '.mjs', '.ts', '.cts', '.mts']); + assert.ok(Object.isFrozen(ANALYZABLE_EXTENSIONS)); + }); + + test('exposes the documented module-scoped constants', () => { + assert.equal(BASELINE_FILE_NAME, 'complexity-baseline.json'); + assert.equal(PROPOSAL_SUFFIX, '-REFACTOR.md'); + assert.equal(SCHEMA_VERSION, 1); + }); +}); diff --git a/tests/refactor-trigger-cli.test.cjs b/tests/refactor-trigger-cli.test.cjs new file mode 100644 index 000000000..faab531c2 --- /dev/null +++ b/tests/refactor-trigger-cli.test.cjs @@ -0,0 +1,981 @@ +'use strict'; + +/** + * Refactor-trigger integration + e2e tests (issue #1953). + * + * Covers test matrix rows 54b and 55-92 — the git adapter, the CLI contract + * (`gsd-tools refactor `), disposition + ledger interplay, strict + * mode's broken-windows integration, and the loop/registry wiring. Rows 1-54 + * (analyzer, evaluator, baseline persistence) live in + * tests/complexity-trigger.test.cjs — out of scope here. + * + * Spec sources (authoritative): + * - .gsd/phase/feat-1953-complexity-triggered-refactor/42-router-contract.md + * - .gsd/phase/feat-1953-complexity-triggered-refactor/50-test-matrix.md + * - .gsd/phase/feat-1953-complexity-triggered-refactor/40-design.md + * + * Automation split (per 50-test-matrix.md Step 1): rows 55-56/61-62/74-90 use + * the router's `_git`/`_windows`/`_core` injection seams in-process (no real + * git/broken-windows I/O, no temp-dir races); rows 57-60/63-73 exercise the + * real `gsd-tools` CLI subprocess via tests/helpers/process-seam.cjs, because + * they are the actual CLI-contract/subprocess-degrade surface. + * + * Hermetic: every fixture dir via createTempGitProject/createTempDir + + * t.after(cleanup); every fs/require mock via mock.method(...), restored via + * t.after(...mock.restore()). No shared module-level fixture state (row 91). + * Every gsd-tools subprocess invocation clears GSD_WORKSTREAM/GSD_PROJECT/ + * GSD_SESSION_KEY so a developer's shell cannot redirect the fixture (row 92). + */ + +const { describe, test, mock } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const Module = require('node:module'); + +const { createTempGitProject, createTempDir, cleanup } = require('./helpers.cjs'); +const { gitOrThrow } = require('./helpers/git-fixture.cjs'); +const { runNode, OUTCOME } = require('./helpers/process-seam.cjs'); + +const { + VERDICT, + REASON, + PROPOSAL_SUFFIX, +} = require('../gsd-core/bin/lib/complexity-trigger.cjs'); +const gitBaseBranch = require('../gsd-core/bin/lib/git-base-branch.cjs'); +const windowsModule = require('../gsd-core/bin/lib/broken-windows.cjs'); +const { routeRefactorTriggerCommand } = require('../gsd-core/bin/lib/refactor-trigger-command-router.cjs'); +const registry = require('../gsd-core/bin/lib/capability-registry.cjs'); +const { validateCapability, VALID_LOOP_POINTS } = require('../gsd-core/bin/lib/capability-validator.cjs'); +const { resolveLoopHooks } = require('../gsd-core/bin/lib/loop-resolver.cjs'); +const { ERROR_REASON } = require('../gsd-core/bin/lib/io.cjs'); +const refactorTriggerCapability = require('../capabilities/refactor-trigger/capability.json'); + +const GSD_TOOLS = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); +const ROUTER_MODULE_PATH = require.resolve('../gsd-core/bin/lib/refactor-trigger-command-router.cjs'); + +// A `gsd-tools refactor` subprocess in this suite does module require plus one +// or two bounded (<=15s) git calls against a tiny mkdtemp repo — well over any +// observed duration for that class of call (mirrors the reasoning in +// tests/helpers/timeouts.cjs's own per-class constants; this call class is not +// one of the four shared there, so it keeps its own local constant per that +// module's documented convention). +const CLI_TIMEOUT_MS = 30000; + +const FLAT_JS = ['function f() {', ' return 1;', '}', ''].join('\n'); +const TRIGGERING_JS = [ + 'function f(x) {', + ' if (x) {', + ' return 1;', + ' }', + ' return 2;', + '}', + '', +].join('\n'); + +function noThrowError(label) { + return (message, reason) => { + throw new Error(`${label}: unexpected error() call — message=${message} reason=${reason}`); + }; +} + +function writeConfig(dir, cfg) { + fs.mkdirSync(path.join(dir, '.planning'), { recursive: true }); + fs.writeFileSync(path.join(dir, '.planning', 'config.json'), JSON.stringify(cfg), 'utf8'); +} + +/** Creates `.planning/phases/01-feat/01-PLAN.md` and commits it — this commit + * becomes the phase-start anchor per the router contract's "Touched-file + * anchor" rule (the commit that ADDED the phase's PLAN.md). */ +function seedPhaseAndAnchor(dir) { + const phaseDir = path.join(dir, '.planning', 'phases', '01-feat'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-PLAN.md'), ['# Plan', ''].join('\n'), 'utf8'); + gitOrThrow(['add', '-A'], { cwd: dir }); + gitOrThrow(['commit', '-m', 'plan'], { cwd: dir }); + return phaseDir; +} + +function commitFile(dir, relPath, content, message) { + const full = path.join(dir, relPath); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, content, 'utf8'); + gitOrThrow(['add', '-A'], { cwd: dir }); + gitOrThrow(['commit', '-m', message], { cwd: dir }); +} + +function runCliOnce(args, cwd, envOverrides = {}) { + return runNode([GSD_TOOLS, ...args], { + cwd, + timeoutMs: CLI_TIMEOUT_MS, + env: { + ...process.env, + // Row 92: clear ambient GSD_* env so a developer's shell cannot + // redirect the fixture. + GSD_WORKSTREAM: '', + GSD_PROJECT: '', + GSD_SESSION_KEY: '', + ...envOverrides, + }, + }); +} + +function assertExited(result, label) { + assert.strictEqual(result.outcome, OUTCOME.EXITED, `${label}: expected EXITED, got outcome=${result.outcome} stderr=${result.stderr}`); +} + +function parseStdout(result) { + return JSON.parse(result.stdout.trim()); +} + +function parseStderr(result) { + return JSON.parse(result.stderr.trim()); +} + +/** Sets up a triggering fixture: strict per `strict`, threshold=1 (any single + * decision point triggers), jumpDelta=100 (never independently triggers). */ +function setupTriggeringProject(prefix, strict) { + const dir = createTempGitProject(prefix); + writeConfig(dir, { + refactor: { + trigger_enabled: true, + trigger_strict: strict, + complexity_threshold: 1, + complexity_jump_delta: 100, + }, + }); + seedPhaseAndAnchor(dir); + commitFile(dir, 'hot.js', TRIGGERING_JS, 'hot file'); + return dir; +} + +// ─── Row 54b — router orchestration continues past an unreadable file ─────── + +describe('refactor-trigger router: continues analyzing after one unreadable touched file (row 54b)', () => { + test('continuesAnalyzingAfterUnreadableFile', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-54b-'); + t.after(() => cleanup(dir)); + writeConfig(dir, { refactor: { trigger_enabled: true, complexity_threshold: 1, complexity_jump_delta: 100 } }); + seedPhaseAndAnchor(dir); + const goodAbs = path.join(dir, 'good.js'); + const badAbs = path.join(dir, 'bad.js'); + fs.writeFileSync(goodAbs, TRIGGERING_JS, 'utf8'); + fs.writeFileSync(badAbs, FLAT_JS, 'utf8'); + gitOrThrow(['add', '-A'], { cwd: dir }); + gitOrThrow(['commit', '-m', 'good and bad'], { cwd: dir }); + + const origReadFileSync = fs.readFileSync; + const readMock = mock.method(fs, 'readFileSync', (target, ...rest) => { + if (target === badAbs) { + const err = new Error('simulated denied read'); + err.code = 'EACCES'; + throw err; + } + return origReadFileSync.call(fs, target, ...rest); + }); + t.after(() => readMock.mock.restore()); + + const outputs = []; + routeRefactorTriggerCommand({ + args: ['refactor', 'evaluate', '--phase', '1', '--raw'], + cwd: dir, + raw: true, + error: noThrowError('row54b'), + _core: { output: (v) => outputs.push(v) }, + }); + + assert.strictEqual(outputs.length, 1); + const result = outputs[0]; + assert.strictEqual(result.verdict, VERDICT.TRIGGERED); + assert.strictEqual(result.target.file, 'good.js'); + assert.strictEqual(result.skipped.length, 1); + assert.strictEqual(result.skipped[0].file, 'bad.js'); + assert.strictEqual(result.skipped[0].reason, REASON.REFACTOR_FILE_UNREADABLE); + }); +}); + +// ─── Rows 55-62 — touched-file discovery (git adapter) ────────────────────── + +describe('refactor-trigger: touched-file discovery (git adapter)', () => { + test('returnsTouchedPathsFromDiff', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-55-'); + t.after(() => cleanup(dir)); + const before = gitOrThrow(['rev-parse', 'HEAD'], { cwd: dir }).trim(); + commitFile(dir, 'a.js', FLAT_JS, 'add a'); + commitFile(dir, 'sub/b.js', FLAT_JS, 'add b'); + commitFile(dir, 'c.cjs', FLAT_JS, 'add c'); + + const touched = gitBaseBranch.changedFilesSince(dir, before); + assert.ok(Array.isArray(touched)); + assert.deepEqual([...touched].sort(), ['a.js', 'c.cjs', 'sub/b.js'].sort()); + }); + + test('handlesEmptyTouchedSet', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-56-'); + t.after(() => cleanup(dir)); + writeConfig(dir, { refactor: { trigger_enabled: true } }); + seedPhaseAndAnchor(dir); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assertExited(result, 'row56'); + assert.strictEqual(result.exitCode, 0); + const parsed = parseStdout(result); + assert.strictEqual(parsed.verdict, VERDICT.BELOW_THRESHOLD); + assert.strictEqual(parsed.reason, REASON.REFACTOR_NO_TOUCHED_FILES); + assert.strictEqual(fs.existsSync(path.join(dir, '.planning', 'phases', '01-feat', `01${PROPOSAL_SUFFIX}`)), false); + }); + + test('degradesWhenNotAGitRepository', (t) => { + const dir = createTempDir('gsd-refactor-cli-57-'); + t.after(() => cleanup(dir)); + writeConfig(dir, { refactor: { trigger_enabled: true } }); + const phaseDir = path.join(dir, '.planning', 'phases', '01-feat'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-PLAN.md'), ['# Plan', ''].join('\n'), 'utf8'); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assertExited(result, 'row57'); + assert.strictEqual(result.exitCode, 0); + const parsed = parseStdout(result); + assert.strictEqual(parsed.verdict, VERDICT.SKIPPED); + assert.strictEqual(parsed.reason, REASON.REFACTOR_GIT_UNAVAILABLE); + assert.strictEqual(fs.existsSync(path.join(phaseDir, `01${PROPOSAL_SUFFIX}`)), false); + }); + + test('degradesWhenGitBinaryMissing', () => { + const enoentExecGit = () => ({ + exitCode: 127, + stdout: '', + stderr: 'git: not found', + signal: null, + error: Object.assign(new Error('spawn git ENOENT'), { code: 'ENOENT' }), + timedOut: false, + }); + assert.strictEqual(gitBaseBranch.changedFilesSince('/does-not-matter', 'HEAD~1', enoentExecGit), null); + assert.strictEqual(gitBaseBranch.phaseStartCommit('/does-not-matter', '.planning/phases/01-feat', enoentExecGit), null); + }); + + test('degradesWhenGitTimesOut', () => { + const timeoutExecGit = () => ({ + exitCode: 1, + stdout: '', + stderr: '', + signal: null, + error: Object.assign(new Error('spawnSync git ETIMEDOUT'), { code: 'ETIMEDOUT' }), + timedOut: true, + }); + assert.strictEqual(gitBaseBranch.changedFilesSince('/does-not-matter', 'HEAD~1', timeoutExecGit), null); + assert.strictEqual(gitBaseBranch.phaseStartCommit('/does-not-matter', '.planning/phases/01-feat', timeoutExecGit), null); + }); + + test('roundTripsHostilePathsViaNulSeparatedDiff', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-60-'); + t.after(() => cleanup(dir)); + const before = gitOrThrow(['rev-parse', 'HEAD'], { cwd: dir }).trim(); + const spaceName = 'a file with spaces.js'; + const unicodeName = 'café-日本.js'; + const dashLeadingName = '--leading-dash.js'; + commitFile(dir, spaceName, FLAT_JS, 'space name'); + commitFile(dir, unicodeName, FLAT_JS, 'unicode name'); + commitFile(dir, dashLeadingName, FLAT_JS, 'dash-leading name'); + + const touched = gitBaseBranch.changedFilesSince(dir, before); + assert.ok(Array.isArray(touched)); + assert.deepEqual([...touched].sort(), [spaceName, unicodeName, dashLeadingName].sort()); + }); + + test('doesNotInterpretFlagLikePathAsGitOption', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-61-'); + t.after(() => cleanup(dir)); + const before = gitOrThrow(['rev-parse', 'HEAD'], { cwd: dir }).trim(); + const flagLikeName = '--upload-pack=evil.js'; + commitFile(dir, flagLikeName, FLAT_JS, 'flag-like name'); + + const touched = gitBaseBranch.changedFilesSince(dir, before); + assert.ok(Array.isArray(touched)); + assert.deepEqual(touched, [flagLikeName]); + }); + + test('refusesPathEscapingRepoRoot', (t) => { + const dir = createTempDir('gsd-refactor-cli-62-'); + t.after(() => cleanup(dir)); + writeConfig(dir, { refactor: { trigger_enabled: true } }); + const phaseDir = path.join(dir, '.planning', 'phases', '01-feat'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-PLAN.md'), ['# Plan', ''].join('\n'), 'utf8'); + + // A real, readable file OUTSIDE the project root that a traversal path + // could reach if the router failed to confine it. + const secretParent = createTempDir('gsd-refactor-cli-62-secret-'); + t.after(() => cleanup(secretParent)); + const secretFile = path.join(secretParent, 'escaped.js'); + fs.writeFileSync(secretFile, TRIGGERING_JS, 'utf8'); + const traversal = path.relative(dir, secretFile); + + const readCalls = []; + const origReadFileSync = fs.readFileSync; + const readMock = mock.method(fs, 'readFileSync', (target, ...rest) => { + readCalls.push(target); + return origReadFileSync.call(fs, target, ...rest); + }); + t.after(() => readMock.mock.restore()); + + const outputs = []; + routeRefactorTriggerCommand({ + args: ['refactor', 'evaluate', '--phase', '1', '--raw'], + cwd: dir, + raw: true, + error: noThrowError('row62'), + _git: { + phaseStartCommit: () => 'HEAD~1', + changedFilesSince: () => [traversal], + }, + _core: { output: (v) => outputs.push(v) }, + }); + + assert.strictEqual(outputs.length, 1); + const result = outputs[0]; + assert.strictEqual(result.verdict, VERDICT.BELOW_THRESHOLD); + assert.strictEqual(result.skipped.length, 1); + assert.strictEqual(result.skipped[0].reason, REASON.REFACTOR_FILE_UNREADABLE); + assert.strictEqual(readCalls.includes(secretFile), false, 'the escaping path must never reach fs.readFileSync'); + }); +}); + +// ─── Rows 63-73 — CLI contract: `gsd-tools refactor` ───────────────────────── + +describe('refactor-trigger: CLI contract — gsd-tools refactor', () => { + test('writesProposalArtifactForTriggeringPhase', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-63-', false); + t.after(() => cleanup(dir)); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assertExited(result, 'row63'); + assert.strictEqual(result.exitCode, 0); + const parsed = parseStdout(result); + assert.strictEqual(parsed.verdict, VERDICT.TRIGGERED); + assert.strictEqual(parsed.artifact_written, true); + assert.strictEqual(typeof parsed.artifact_path, 'string'); + assert.strictEqual(fs.statSync(parsed.artifact_path).isFile(), true); + }); + + test('writesNoArtifactBelowThreshold', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-64-'); + t.after(() => cleanup(dir)); + writeConfig(dir, { refactor: { trigger_enabled: true, complexity_threshold: 1000, complexity_jump_delta: 1000 } }); + seedPhaseAndAnchor(dir); + commitFile(dir, 'flat.js', FLAT_JS, 'flat file'); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assertExited(result, 'row64'); + assert.strictEqual(result.exitCode, 0); + const parsed = parseStdout(result); + assert.strictEqual(parsed.verdict, VERDICT.BELOW_THRESHOLD); + assert.strictEqual(parsed.artifact_written, false); + assert.strictEqual(parsed.artifact_path, null); + assert.strictEqual(fs.existsSync(path.join(dir, '.planning', 'phases', '01-feat', `01${PROPOSAL_SUFFIX}`)), false); + }); + + test('returnsDisabledResponseWhenCapabilityOff', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-65-'); + t.after(() => cleanup(dir)); + seedPhaseAndAnchor(dir); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assertExited(result, 'row65'); + assert.strictEqual(result.exitCode, 0); + const parsed = parseStdout(result); + assert.strictEqual(parsed.disabled, true); + }); + + test('rejectsMissingPhaseArgument', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-66-', false); + t.after(() => cleanup(dir)); + + const result = runCliOnce(['refactor', 'evaluate', '--raw'], dir, { GSD_JSON_ERRORS: '1' }); + assertExited(result, 'row66'); + assert.notStrictEqual(result.exitCode, 0); + const parsed = parseStderr(result); + assert.strictEqual(parsed.ok, false); + assert.strictEqual(parsed.reason, REASON.REFACTOR_USAGE); + }); + + test('rejectsEmptyAndWhitespacePhase', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-67-', false); + t.after(() => cleanup(dir)); + + for (const phaseValue of ['', ' ']) { + const result = runCliOnce(['refactor', 'evaluate', '--phase', phaseValue, '--raw'], dir, { GSD_JSON_ERRORS: '1' }); + assertExited(result, `row67(${JSON.stringify(phaseValue)})`); + assert.notStrictEqual(result.exitCode, 0); + const parsed = parseStderr(result); + assert.strictEqual(parsed.reason, REASON.REFACTOR_INVALID_PHASE); + } + }); + + // Matrix row 68 names REFACTOR_USAGE as the expected reason; the router + // contract (42-router-contract.md "evaluate" step 2) and the shipped + // implementation both treat `--phase=`/`--phase==N` as PRESENT-BUT-INVALID + // (REFACTOR_INVALID_PHASE), reserving REFACTOR_USAGE for the flag being + // absent entirely. Asserted against the router contract + implementation, + // which agree with each other; the matrix wording is the stale one here — + // see the final report for this discrepancy. + test('rejectsMalformedPhaseAssignment', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-68-', false); + t.after(() => cleanup(dir)); + + for (const arg of ['--phase=', '--phase==3']) { + const result = runCliOnce(['refactor', 'evaluate', arg, '--raw'], dir, { GSD_JSON_ERRORS: '1' }); + assertExited(result, `row68(${arg})`); + assert.notStrictEqual(result.exitCode, 0); + const parsed = parseStderr(result); + assert.strictEqual(parsed.reason, REASON.REFACTOR_INVALID_PHASE); + } + }); + + test('rejectsDuplicatePhaseFlag', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-69-', false); + t.after(() => cleanup(dir)); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--phase', '2', '--raw'], dir, { GSD_JSON_ERRORS: '1' }); + assertExited(result, 'row69'); + assert.notStrictEqual(result.exitCode, 0); + const parsed = parseStderr(result); + assert.strictEqual(parsed.reason, REASON.REFACTOR_INVALID_PHASE); + }); + + test('rejectsFlagLikePhaseValue', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-70-', false); + t.after(() => cleanup(dir)); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '--raw'], dir, { GSD_JSON_ERRORS: '1' }); + assertExited(result, 'row70'); + assert.notStrictEqual(result.exitCode, 0); + const parsed = parseStderr(result); + assert.strictEqual(parsed.reason, REASON.REFACTOR_INVALID_PHASE); + }); + + test('reportsUnknownSubcommandWithAvailableList', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-71-', false); + t.after(() => cleanup(dir)); + + const result = runCliOnce(['refactor', 'frobnicate', '--raw'], dir, { GSD_JSON_ERRORS: '1' }); + assertExited(result, 'row71'); + assert.notStrictEqual(result.exitCode, 0); + const parsed = parseStderr(result); + assert.strictEqual(parsed.reason, ERROR_REASON.SDK_UNKNOWN_COMMAND); + }); + + test('rejectsOverlongAndUnicodePhaseValues', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-72-', false); + t.after(() => cleanup(dir)); + + // Overlong: matches the positive-integer regex (all digits), so it clears + // arg validation but fails to resolve as a real phase directory -> + // REFACTOR_INVALID_PHASE via the resolve-phase-dir path (exit 0, typed + // verdict on stdout) rather than the arg-validation path. + const overlong = '9'.repeat(5000); + const overlongResult = runCliOnce(['refactor', 'evaluate', '--phase', overlong, '--raw'], dir); + assertExited(overlongResult, 'row72(overlong)'); + assert.strictEqual(overlongResult.exitCode, 0); + const overlongParsed = parseStdout(overlongResult); + assert.strictEqual(overlongParsed.verdict, VERDICT.SKIPPED); + assert.strictEqual(overlongParsed.reason, REASON.REFACTOR_INVALID_PHASE); + + // Unicode: fails the arg-validation regex outright. + const unicodeResult = runCliOnce(['refactor', 'evaluate', '--phase', '一二三', '--raw'], dir, { GSD_JSON_ERRORS: '1' }); + assertExited(unicodeResult, 'row72(unicode)'); + assert.notStrictEqual(unicodeResult.exitCode, 0); + const unicodeParsed = parseStderr(unicodeResult); + assert.strictEqual(unicodeParsed.reason, REASON.REFACTOR_INVALID_PHASE); + }); + + test('provesNoShellInterpolationOfPhaseValue', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-73-', false); + t.after(() => cleanup(dir)); + const sentinel = path.join(dir, 'SENTINEL'); + + const payloads = [ + `1;touch ${sentinel};`, + `$(touch ${sentinel})`, + '`touch ' + sentinel + '`', + ]; + for (const payload of payloads) { + const result = runCliOnce(['refactor', 'evaluate', '--phase', payload, '--raw'], dir, { GSD_JSON_ERRORS: '1' }); + assertExited(result, `row73(${JSON.stringify(payload)})`); + assert.notStrictEqual(result.exitCode, 0); + assert.strictEqual(fs.existsSync(sentinel), false, `sentinel must not exist after payload ${JSON.stringify(payload)}`); + const parsed = parseStderr(result); + assert.strictEqual(parsed.reason, REASON.REFACTOR_INVALID_PHASE); + } + }); +}); + +// ─── Rows 74-79 — disposition + ledger ─────────────────────────────────────── + +describe('refactor-trigger: disposition + ledger', () => { + test('recordsDeclinedRefactorAsDeviationWindow', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-74-', true); + t.after(() => cleanup(dir)); + const evalResult = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(evalResult.exitCode, 0); + assert.strictEqual(parseStdout(evalResult).ledger_recorded, true); + + const declineResult = runCliOnce(['refactor', 'decline', '--phase', '1', '--reason', 'not worth it', '--raw'], dir); + assertExited(declineResult, 'row74'); + assert.strictEqual(declineResult.exitCode, 0); + const declineParsed = parseStdout(declineResult); + assert.strictEqual(declineParsed.status, 'declined'); + + const ledgerPath = path.join(dir, '.planning', windowsModule.LEDGER_FILE_NAME); + const ledger = windowsModule.parseLedger(fs.readFileSync(ledgerPath, 'utf8')); + assert.strictEqual(ledger.entries.length, 1); + assert.strictEqual(ledger.entries[0].kind, 'deviation'); + assert.strictEqual(ledger.entries[0].status, 'waived'); + }); + + test('declinesGracefullyWithoutBrokenWindows', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-75-'); + t.after(() => cleanup(dir)); + writeConfig(dir, { refactor: { trigger_enabled: true, trigger_strict: false, complexity_threshold: 1, complexity_jump_delta: 100 } }); + seedPhaseAndAnchor(dir); + commitFile(dir, 'hot.js', TRIGGERING_JS, 'hot file'); + + const evalOutputs = []; + routeRefactorTriggerCommand({ + args: ['refactor', 'evaluate', '--phase', '1', '--raw'], + cwd: dir, + raw: true, + error: noThrowError('row75(evaluate)'), + _core: { output: (v) => evalOutputs.push(v) }, + }); + assert.strictEqual(evalOutputs[0].verdict, VERDICT.TRIGGERED); + + // Simulate broken-windows genuinely absent: make the router's own + // `require('./broken-windows.cjs')` throw, exactly as the router contract + // documents ("absent or a throwing require is the documented degrade + // path"). Scoped to this module's own require calls only. + const origRequire = Module.prototype.require; + const requireMock = mock.method(Module.prototype, 'require', function mockedRequire(id) { + if (id === './broken-windows.cjs' && this.filename === ROUTER_MODULE_PATH) { + throw new Error('simulated: broken-windows capability not installed'); + } + return origRequire.call(this, id); + }); + t.after(() => requireMock.mock.restore()); + + const declineOutputs = []; + routeRefactorTriggerCommand({ + args: ['refactor', 'decline', '--phase', '1', '--reason', 'not now', '--raw'], + cwd: dir, + raw: true, + error: noThrowError('row75(decline)'), + _core: { output: (v) => declineOutputs.push(v) }, + }); + + assert.strictEqual(declineOutputs.length, 1); + const declineResult = declineOutputs[0]; + assert.strictEqual(declineResult.status, 'declined'); + assert.strictEqual(declineResult.ledger_resolved, false); + assert.strictEqual(typeof declineResult.ledger_note, 'string'); + assert.notStrictEqual(declineResult.ledger_note, ''); + assert.strictEqual(fs.existsSync(path.join(dir, '.planning', windowsModule.LEDGER_FILE_NAME)), false); + }); + + test('rejectsEmptyDeclineReason', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-76-', false); + t.after(() => cleanup(dir)); + const evalResult = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(evalResult.exitCode, 0); + const artifactPath = parseStdout(evalResult).artifact_path; + + for (const reason of ['', ' ']) { + const result = runCliOnce(['refactor', 'decline', '--phase', '1', '--reason', reason, '--raw'], dir, { GSD_JSON_ERRORS: '1' }); + assertExited(result, `row76(${JSON.stringify(reason)})`); + assert.notStrictEqual(result.exitCode, 0); + const parsed = parseStderr(result); + assert.strictEqual(parsed.reason, REASON.REFACTOR_DECLINE_REASON_EMPTY); + } + + const { parseProposal } = require('../gsd-core/bin/lib/complexity-trigger.cjs'); + const proposal = parseProposal(fs.readFileSync(artifactPath, 'utf8')); + assert.strictEqual(proposal.status, 'proposed', 'a rejected decline must not change the proposal status'); + }); + + test('doesNotAppendDuplicateWindowOnRepeatedDecline', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-77-', true); + t.after(() => cleanup(dir)); + const evalResult = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(evalResult.exitCode, 0); + + const first = runCliOnce(['refactor', 'decline', '--phase', '1', '--reason', 'first', '--raw'], dir); + assert.strictEqual(first.exitCode, 0); + + const second = runCliOnce(['refactor', 'decline', '--phase', '1', '--reason', 'second', '--raw'], dir, { GSD_JSON_ERRORS: '1' }); + assertExited(second, 'row77(second)'); + assert.notStrictEqual(second.exitCode, 0); + assert.strictEqual(parseStderr(second).reason, REASON.REFACTOR_ALREADY_DISPOSITIONED); + + const ledgerPath = path.join(dir, '.planning', windowsModule.LEDGER_FILE_NAME); + const ledger = windowsModule.parseLedger(fs.readFileSync(ledgerPath, 'utf8')); + assert.strictEqual(ledger.entries.length, 1); + }); + + test('rejectsDeclineWithNoProposal', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-78-'); + t.after(() => cleanup(dir)); + writeConfig(dir, { refactor: { trigger_enabled: true } }); + seedPhaseAndAnchor(dir); + + const result = runCliOnce(['refactor', 'decline', '--phase', '1', '--reason', 'x', '--raw'], dir, { GSD_JSON_ERRORS: '1' }); + assertExited(result, 'row78'); + assert.notStrictEqual(result.exitCode, 0); + assert.strictEqual(parseStderr(result).reason, REASON.REFACTOR_ARTIFACT_NOT_FOUND); + }); + + test('treatsAcceptAndDeclineAsMutuallyExclusive', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-79-', false); + t.after(() => cleanup(dir)); + const evalResult = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(evalResult.exitCode, 0); + + const acceptResult = runCliOnce(['refactor', 'accept', '--phase', '1', '--raw'], dir); + assert.strictEqual(acceptResult.exitCode, 0); + assert.strictEqual(parseStdout(acceptResult).status, 'accepted'); + + const declineResult = runCliOnce(['refactor', 'decline', '--phase', '1', '--reason', 'x', '--raw'], dir, { GSD_JSON_ERRORS: '1' }); + assertExited(declineResult, 'row79'); + assert.notStrictEqual(declineResult.exitCode, 0); + assert.strictEqual(parseStderr(declineResult).reason, REASON.REFACTOR_ALREADY_DISPOSITIONED); + }); +}); + +// ─── Rows 80-86 — strict mode -> broken-windows ledger ─────────────────────── + +describe('refactor-trigger: strict mode -> broken-windows ledger', () => { + test('appendsNoWindowInAdvisoryMode', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-80-', false); + t.after(() => cleanup(dir)); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(result.exitCode, 0); + const parsed = parseStdout(result); + assert.strictEqual(parsed.verdict, VERDICT.TRIGGERED); + assert.strictEqual(parsed.artifact_written, true); + assert.strictEqual(parsed.ledger_recorded, undefined); + assert.strictEqual(fs.existsSync(path.join(dir, '.planning', windowsModule.LEDGER_FILE_NAME)), false); + }); + + test('appendsNoWindowWhenNothingTriggered', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-81-'); + t.after(() => cleanup(dir)); + writeConfig(dir, { + refactor: { trigger_enabled: true, trigger_strict: true, complexity_threshold: 1000, complexity_jump_delta: 1000 }, + }); + seedPhaseAndAnchor(dir); + commitFile(dir, 'flat.js', FLAT_JS, 'flat file'); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(result.exitCode, 0); + const parsed = parseStdout(result); + assert.strictEqual(parsed.verdict, VERDICT.BELOW_THRESHOLD); + assert.strictEqual(parsed.artifact_written, false); + assert.strictEqual(parsed.ledger_recorded, undefined, 'F1 regression guard: nothing triggered => zero windows, ever'); + assert.strictEqual(fs.existsSync(path.join(dir, '.planning', windowsModule.LEDGER_FILE_NAME)), false); + }); + + test('appendsDeviationWindowForUntriagedProposal', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-82-', true); + t.after(() => cleanup(dir)); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(result.exitCode, 0); + assert.strictEqual(parseStdout(result).ledger_recorded, true); + + const ledgerPath = path.join(dir, '.planning', windowsModule.LEDGER_FILE_NAME); + const ledger = windowsModule.parseLedger(fs.readFileSync(ledgerPath, 'utf8')); + assert.strictEqual(ledger.open_count, 1); + assert.strictEqual(ledger.entries.length, 1); + const entry = ledger.entries[0]; + assert.strictEqual(entry.kind, 'deviation'); + assert.strictEqual(entry.status, 'open'); + assert.strictEqual(entry.file, 'hot.js', 'dedup identity is the structured file field, not prose'); + assert.strictEqual(entry.line, 1, 'dedup identity is the structured line field (target function start line)'); + }); + + test('resolvesWindowOnAcceptRegardlessOfScore', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-83-', true); + t.after(() => cleanup(dir)); + + const evalResult = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(evalResult.exitCode, 0); + const evalParsed = parseStdout(evalResult); + assert.strictEqual(evalParsed.verdict, VERDICT.TRIGGERED); + const triggeringScore = evalParsed.target.score; + + // Accept WITHOUT touching hot.js at all — its live score cannot have + // improved, and per config it is still strictly above the threshold. + const acceptResult = runCliOnce(['refactor', 'accept', '--phase', '1', '--raw'], dir); + assert.strictEqual(acceptResult.exitCode, 0); + const acceptParsed = parseStdout(acceptResult); + assert.strictEqual(acceptParsed.reanchored_to, triggeringScore, 'the Goodhart guarantee: the score did not improve'); + assert.ok(acceptParsed.reanchored_to > 1, 'score must still be above the configured threshold (1)'); + assert.strictEqual(acceptParsed.ledger_resolved, true); + + const ledgerPath = path.join(dir, '.planning', windowsModule.LEDGER_FILE_NAME); + const ledger = windowsModule.parseLedger(fs.readFileSync(ledgerPath, 'utf8')); + assert.strictEqual(ledger.open_count, 0); + assert.strictEqual(ledger.fixed_count, 1); + }); + + test('waivesWindowWithReasonOnDecline', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-84-', true); + t.after(() => cleanup(dir)); + const evalResult = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(evalResult.exitCode, 0); + + const reasonText = 'deferred to next phase'; + const declineResult = runCliOnce(['refactor', 'decline', '--phase', '1', '--reason', reasonText, '--raw'], dir); + assert.strictEqual(declineResult.exitCode, 0); + assert.strictEqual(parseStdout(declineResult).ledger_resolved, true); + + const ledgerPath = path.join(dir, '.planning', windowsModule.LEDGER_FILE_NAME); + const ledger = windowsModule.parseLedger(fs.readFileSync(ledgerPath, 'utf8')); + assert.strictEqual(ledger.entries.length, 1); + assert.strictEqual(ledger.entries[0].status, 'waived'); + assert.strictEqual(ledger.entries[0].reason, reasonText); + }); + + test('doesNotDuplicateWindowOnReEvaluate', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-85-', true); + t.after(() => cleanup(dir)); + + const first = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(first.exitCode, 0); + const second = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(second.exitCode, 0); + assert.strictEqual(parseStdout(second).ledger_recorded, true); + + const ledgerPath = path.join(dir, '.planning', windowsModule.LEDGER_FILE_NAME); + const ledger = windowsModule.parseLedger(fs.readFileSync(ledgerPath, 'utf8')); + assert.strictEqual(ledger.entries.length, 1); + assert.strictEqual(ledger.open_count, 1); + }); + + test('notesLedgerUnavailableWithoutBrokenWindows', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-86-'); + t.after(() => cleanup(dir)); + writeConfig(dir, { + refactor: { trigger_enabled: true, trigger_strict: true, complexity_threshold: 1, complexity_jump_delta: 100 }, + }); + seedPhaseAndAnchor(dir); + commitFile(dir, 'hot.js', TRIGGERING_JS, 'hot file'); + + const origRequire = Module.prototype.require; + const requireMock = mock.method(Module.prototype, 'require', function mockedRequire(id) { + if (id === './broken-windows.cjs' && this.filename === ROUTER_MODULE_PATH) { + throw new Error('simulated: broken-windows capability not installed'); + } + return origRequire.call(this, id); + }); + t.after(() => requireMock.mock.restore()); + + const outputs = []; + routeRefactorTriggerCommand({ + args: ['refactor', 'evaluate', '--phase', '1', '--raw'], + cwd: dir, + raw: true, + error: noThrowError('row86'), + _core: { output: (v) => outputs.push(v) }, + }); + + assert.strictEqual(outputs.length, 1); + const result = outputs[0]; + assert.strictEqual(result.verdict, VERDICT.TRIGGERED); + assert.strictEqual(result.artifact_written, true); + assert.strictEqual(result.ledger_recorded, false); + assert.strictEqual(result.ledger_note, 'broken-windows capability unavailable — proposal recorded locally only'); + assert.strictEqual(fs.existsSync(path.join(dir, '.planning', windowsModule.LEDGER_FILE_NAME)), false); + }); +}); + +// ─── Strict-mode enforcement-gap warning (#1953) ───────────────────────────── +// +// `refactor.trigger_strict` only ever appends a deviation window to the +// broken-windows ledger; a ship only actually stops when the SEPARATE +// `workflow.windows_enforce` toggle is also on. These cases lock the typed +// `REFACTOR_STRICT_NOT_ENFORCING` warning that closes that expectation gap. + +describe('refactor-trigger: strict-mode enforcement-gap warning', () => { + test('warnsWhenStrictOnAndWindowsEnforceOff', (t) => { + const dir = setupTriggeringProject('gsd-refactor-cli-swe-1-', true); + t.after(() => cleanup(dir)); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(result.exitCode, 0); + const parsed = parseStdout(result); + assert.strictEqual(parsed.ledger_recorded, true, 'broken-windows must be present and recording for this case'); + assert.ok(Array.isArray(parsed.warnings)); + assert.strictEqual(parsed.warnings.length, 1); + assert.strictEqual(parsed.warnings[0].reason, REASON.REFACTOR_STRICT_NOT_ENFORCING); + assert.strictEqual(typeof parsed.warnings[0].message, 'string'); + assert.notStrictEqual(parsed.warnings[0].message, ''); + }); + + test('omitsWarningWhenStrictOnAndWindowsEnforceOn', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-swe-2-'); + t.after(() => cleanup(dir)); + writeConfig(dir, { + refactor: { trigger_enabled: true, trigger_strict: true, complexity_threshold: 1, complexity_jump_delta: 100 }, + workflow: { windows_enforce: true }, + }); + seedPhaseAndAnchor(dir); + commitFile(dir, 'hot.js', TRIGGERING_JS, 'hot file'); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(result.exitCode, 0); + const parsed = parseStdout(result); + assert.strictEqual(parsed.ledger_recorded, true, 'broken-windows must be present and recording for this case'); + assert.strictEqual(parsed.warnings, undefined); + }); + + test('warnsWhenStrictOnAndBrokenWindowsAbsent', (t) => { + const dir = createTempGitProject('gsd-refactor-cli-swe-3-'); + t.after(() => cleanup(dir)); + writeConfig(dir, { + refactor: { trigger_enabled: true, trigger_strict: true, complexity_threshold: 1, complexity_jump_delta: 100 }, + }); + seedPhaseAndAnchor(dir); + commitFile(dir, 'hot.js', TRIGGERING_JS, 'hot file'); + + // Simulate broken-windows genuinely absent, exactly as + // `notesLedgerUnavailableWithoutBrokenWindows` (row 86) does. + const origRequire = Module.prototype.require; + const requireMock = mock.method(Module.prototype, 'require', function mockedRequire(id) { + if (id === './broken-windows.cjs' && this.filename === ROUTER_MODULE_PATH) { + throw new Error('simulated: broken-windows capability not installed'); + } + return origRequire.call(this, id); + }); + t.after(() => requireMock.mock.restore()); + + const outputs = []; + routeRefactorTriggerCommand({ + args: ['refactor', 'evaluate', '--phase', '1', '--raw'], + cwd: dir, + raw: true, + error: noThrowError('swe-3'), + _core: { output: (v) => outputs.push(v) }, + }); + + assert.strictEqual(outputs.length, 1); + const result = outputs[0]; + assert.strictEqual(result.ledger_recorded, false); + assert.ok(Array.isArray(result.warnings)); + assert.strictEqual(result.warnings.length, 1); + assert.strictEqual(result.warnings[0].reason, REASON.REFACTOR_STRICT_NOT_ENFORCING); + assert.strictEqual(typeof result.warnings[0].message, 'string'); + assert.notStrictEqual(result.warnings[0].message, ''); + }); + + test('neverWarnsWhenStrictOffRegardlessOfWindowsEnforce', (t) => { + for (const windowsEnforce of [true, false]) { + const dir = createTempGitProject('gsd-refactor-cli-swe-4-'); + t.after(() => cleanup(dir)); + writeConfig(dir, { + refactor: { trigger_enabled: true, trigger_strict: false, complexity_threshold: 1, complexity_jump_delta: 100 }, + workflow: { windows_enforce: windowsEnforce }, + }); + seedPhaseAndAnchor(dir); + commitFile(dir, 'hot.js', TRIGGERING_JS, 'hot file'); + + const result = runCliOnce(['refactor', 'evaluate', '--phase', '1', '--raw'], dir); + assert.strictEqual(result.exitCode, 0); + const parsed = parseStdout(result); + assert.strictEqual(parsed.verdict, VERDICT.TRIGGERED, `windowsEnforce=${windowsEnforce}: fixture must still trigger`); + assert.strictEqual(parsed.ledger_recorded, undefined, `windowsEnforce=${windowsEnforce}: strict is off`); + assert.strictEqual(parsed.warnings, undefined, `windowsEnforce=${windowsEnforce}: strict off must never warn`); + } + }); +}); + +// ─── Row 87 (independence) — registry declares zero ship:pre gates ────────── + +describe('refactor-trigger: registry independence', () => { + test('declaresNoShipPreGate', () => { + const cap = registry.capabilities['refactor-trigger']; + assert.ok(cap, 'refactor-trigger must be present in the registry'); + assert.deepEqual(cap.gates, []); + + const shipPre = registry.byLoopPoint['ship:pre']; + assert.strictEqual(shipPre.gates.length, 2); + assert.deepEqual(shipPre.gates.map((g) => g.capId).sort(), ['broken-windows', 'security']); + }); +}); + +// ─── Rows 87-90 — loop wiring + manifest validation ────────────────────────── + +describe('refactor-trigger: loop wiring', () => { + test('rendersRefactorStepAtExecutePost', () => { + const resolved = resolveLoopHooks({ + point: 'execute:post', + registry, + config: { refactor: { trigger_enabled: true } }, + }); + const step = resolved.activeHooks.find((h) => h.capId === 'refactor-trigger'); + assert.ok(step, 'refactor-trigger step must be present when enabled'); + assert.strictEqual(step.kind, 'step'); + assert.deepEqual(step.ref, { command: 'refactor evaluate' }); + assert.strictEqual(step.when, 'refactor.trigger_enabled'); + assert.strictEqual(step.onError, 'skip'); + }); + + test('omitsRefactorStepWhenDisabled', () => { + const resolved = resolveLoopHooks({ + point: 'execute:post', + registry, + config: { refactor: { trigger_enabled: false } }, + }); + const step = resolved.activeHooks.find((h) => h.capId === 'refactor-trigger'); + assert.strictEqual(step, undefined, 'refactor-trigger step must be absent when disabled'); + }); + + test('preservesCodeReviewHookShapeAlongsideRefactorHook', () => { + const resolved = resolveLoopHooks({ + point: 'execute:post', + registry, + config: { refactor: { trigger_enabled: true } }, + }); + assert.strictEqual(resolved.activeHooks.length, 2, 'both refactor-trigger and code-review must be present'); + const codeReview = resolved.activeHooks.find((h) => h.capId === 'code-review'); + assert.ok(codeReview, 'code-review step must still be present alongside refactor-trigger'); + assert.deepEqual(codeReview, { + capId: 'code-review', + kind: 'step', + ref: { skill: 'code-review' }, + when: 'workflow.code_review', + produces: ['REVIEW.md'], + consumes: ['SUMMARY.md'], + onError: 'skip', + }); + }); + + test('validatesRefactorTriggerManifest', () => { + const errors = validateCapability(refactorTriggerCapability, 'refactor-trigger'); + assert.deepEqual(errors, []); + + const ownConfigKeys = [ + 'refactor.trigger_enabled', + 'refactor.complexity_threshold', + 'refactor.complexity_jump_delta', + 'refactor.trigger_strict', + ]; + for (const key of ownConfigKeys) { + assert.strictEqual(registry.configKeys[key], 'refactor-trigger', `config key ${key} must be owned solely by refactor-trigger, no other capability`); + } + + for (const step of refactorTriggerCapability.steps) { + assert.strictEqual(VALID_LOOP_POINTS.has(step.point), true, `step.point ${step.point} must be one of the closed 12 loop points`); + } + }); +});