Commit Graph

3 Commits

Author SHA1 Message Date
Tom Boucher
b8c33647d8 refactor(tests): retire output-grep & source-grep via typed surfaces (finish #2974) (#462)
* refactor(#455): implement typed surfaces to retire grep tests

Production surfaces added:
- hooks/managed-hooks-registry.cjs: new CJS module exporting MANAGED_HOOKS
  as a typed array; gsd-check-update-worker.js now requires it instead of
  declaring an inline array
- bin/install.js: elevate inline gsdHooks to module-level GSD_UNINSTALL_HOOKS,
  export it alongside runtimeMap/allRuntimes (already exported)
- scripts/build-hooks.js: export HOOKS_TO_COPY; guard build() behind
  require.main===module so tests can require the file without triggering a build
- get-shit-done/bin/lib/init.cjs: add --json mode to agent-skills command,
  emitting typed IR { agent_type, block, skills_count } for test assertions
- get-shit-done/bin/gsd-tools.cjs: wire --json flag for agent-skills dispatch

Category-B source-grep migrations:
- tests/managed-hooks.test.cjs: require MANAGED_HOOKS from registry, drop fs.readFileSync+regex
- tests/orphaned-hooks.test.cjs: require MANAGED_HOOKS+HOOKS_TO_COPY as typed exports
- tests/hooks-opt-in.test.cjs: replace gsdHooks regex-parse with GSD_UNINSTALL_HOOKS import
- tests/install-minimal-hooks.test.cjs: replace gsdHooks regex-parse with GSD_UNINSTALL_HOOKS
- tests/copilot-install.test.cjs: replace src.includes() checks with typed
  assertions on runtimeMap, allRuntimes, parseRuntimeInput, buildRuntimePromptText
- tests/agent-skills.test.cjs: migrate to --json typed IR assertions

pending-migration-to-typed-ir token cleared (87 of 87 files):
- 78 files already had source-text-is-the-product; removed duplicate token
- 5 files already used typed assertions; reclassified or annotated
- 4 files required individual reclassification to source-text-is-the-product
  or architectural-invariant

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(#455): update workflow-guard test to typed GSD_UNINSTALL_HOOKS import; isolate HOME in runtime-launcher (D) test

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(#455): guard install.js main() behind require.main===module so the typed export is require-safe

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(#455): document --json typed surfaces for agent-skills, progress, validate context

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(#455): add changeset fragment for new --json surfaces

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(#455): complete grep migration for files flagged by lint-tests

The branch commit 4e630d99 stripped `allow-test-rule: pending-migration-to-typed-ir`
from ~80 test files without replacing their assertions or adding the correct
exemption annotation. The files were NOT source-grep tests — they read .md
workflow/agent/command/reference files (source-text-is-the-product) or hook
source files for structural invariants (structural-regression-guard). No
assertion logic was changed; only the correct allow-test-rule annotation was
added to each file per CONTRIBUTING.md exception matrix.

73 files: `source-text-is-the-product` — workflow/agent/command/reference .md
7 files:  `structural-regression-guard` — hook .js / bin/install.js structural checks

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: CI Rebase Check <ci@gsd-redux>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-05-29 11:39:52 -04:00
Tom Boucher
918f987a19 feat(#2982): extend no-source-grep lint to catch var-binding readFileSync.includes() (#2985)
* feat(#2982): extend no-source-grep lint to catch var-binding readFileSync.includes()

The base lint (scripts/lint-no-source-grep.cjs) only catches
readFileSync(...).<text-method>() chained directly. The much more
common var-binding form escapes it:

  const src = fs.readFileSync(p, 'utf8');
  // 50 lines later
  if (src.includes('foo')) {}        // ← still grep, lint missed it

Scan of the test suite found ~141 files using this pattern.

Implementation built TDD per #2982 with structured-IR assertions:

  scripts/lint-no-source-grep-extras.cjs
    - detectVarBindingViolations(src) — pure detector, two passes:
      pass 1 collects vars bound from readFileSync, pass 2 finds any
      <var>.<includes|startsWith|endsWith|match|search>( on those vars.
    - detectWrappedAssertOkMatch(src) — flags
      assert.ok(<expr>.match(...)) which escapes the assert.match rule.
    - VIOLATION enum exposes stable codes for tests to assert on.

  scripts/lint-no-source-grep.cjs
    - Wires the new detectors into the existing per-file check; one
      additional violation row per file with the first 3 sample tokens.

  tests/bug-2982-lint-var-binding.test.cjs
    - 13 tests, all assertions on typed VIOLATION enum / structured
      records. Covers all 5 text-match methods, multi-var, no-bind,
      string literal (must NOT trigger), wrapped assert.ok(.match),
      and assert.match (must NOT double-flag).

Migration backlog (#2974 expanded scope):

  - 42 files annotated `// allow-test-rule: source-text-is-the-product`
    (legitimate — they read .md/.json/.yml files whose deployed text
    IS the product)
  - 3 files annotated `// allow-test-rule: pending-migration-to-typed-ir [#2974]`
    (read .cjs/.js source — clear migration debt)
  - 95 files annotated `pending-migration-to-typed-ir [#2974]` with
    `Per-file review may reclassify as source-text-is-the-product
    during migration` (mixed — manual review under #2974)

After this lands the lint reports 0 violations on main; new
violations in PRs surface immediately.

Closes #2982
Refs #2974

* test(#2982): fix truncated test name per CR

The label ended with a bare '(' from a copy-paste mishap. Now reads
'does NOT flag .matchAll(...) — matchAll is not match, so
assert.ok(.matchAll(...)) is not flagged'.

* chore(#2982): add changeset fragment for PR #2985

* chore(#2982): add changeset fragment for PR #2985
2026-05-01 19:50:10 -04:00
Tom Boucher
8b6c44433f fix(#2772): only disable worktree isolation when planned paths touch submodules (#2779)
* fix(#2772): only disable worktree isolation when planned paths touch submodules

The previous guard in execute-phase.md and quick.md unconditionally set
USE_WORKTREES=false whenever .gitmodules existed, penalising every plan in
a submodule project even when no plan touched a submodule path.

Replace with submodule-path parsing + per-plan path intersection:

- Parse SUBMODULE_PATHS once from .gitmodules via
  `git config --file .gitmodules --get-regexp '^submodule\..*\.path$'`.
- In execute-phase.md, intersect SUBMODULE_PATHS with each plan's
  files_modified frontmatter; disable worktree isolation only for plans
  with non-empty intersection. Fall back to safe-disable for that plan
  when files_modified is missing/unparseable, with a log line explaining
  why.
- In quick.md (no pre-declared paths), keep submodule-path parsing and
  document a fail-loud commit-time guard so the executor aborts only when
  it actually stages a submodule path.

Add tests/bug-2772-gitmodules-path-intersection.test.cjs covering both
files: no unconditional disable, submodule paths are parsed, intersection
logic exists in execute-phase, fallback path is documented.

Full suite: 5680 / 5680 pass.

Closes #2772

* test(#2772): replace source-grep with behavioral test of submodule path intersection

* fix(#2772): wire USE_WORKTREES_FOR_PLAN into dispatch + fix glob matcher + add quick.md commit guard

Address CodeRabbit review on PR #2779 — the original fix computed
USE_WORKTREES_FOR_PLAN but never read it, so the per-plan submodule
intersection was dead code. Dispatch sites still branched on the
project-level USE_WORKTREES.

Changes:

1. execute-phase.md (CRITICAL — dispatch wiring): Move per-plan
   computation into execute_waves as sub-step 2.5, run it for each plan
   before its dispatch, and gate all four dispatch sites on
   USE_WORKTREES_FOR_PLAN: worktree-mode header, sequential-mode header,
   "worktrees disabled" sequential rule, and post-wave cleanup. Document
   PLAN_FILES extraction via jq from the phase-plan-index JSON. Track
   WAVE_WORKTREE_PLANS so post-wave cleanup only runs when at least one
   plan in the wave actually used worktrees.

2. Per-plan gate matcher (MAJOR — glob safety): Strip leading "./" and
   trailing "/" from both submodule and planned paths. Match
   bidirectionally (pf inside sm AND sm inside pf). Handle globby
   planned paths like "vendor/**/*.c" by extracting the literal prefix
   before the first glob metachar and re-checking. Wrap the iteration
   in set -f / set +f so glob expansion does not corrupt patterns.
   Extracted the gate (~92 lines) into
   workflows/execute-phase/steps/per-plan-worktree-gate.md to keep
   execute-phase.md under the 1700-line XL budget.

3. quick.md (CRITICAL — fail-loud guard): Inject SUBMODULE_PATHS into
   the executor Task prompt and add a <submodule_commit_guard> bash
   block the executor must run before every git commit. The guard
   inspects staged paths via `git diff --cached --name-only`, normalizes
   paths, and aborts with a clear ABORT message + recovery instruction
   ("re-run with workflow.use_worktrees=false") when any staged path
   falls inside a submodule.

4. tests/bug-2772-gitmodules-path-intersection.test.cjs: 25 tests total.
   Updated GATE_SNIPPET to match the new bash matcher. Added
   normalization tests (./ prefix, trailing /, glob "vendor/**/*.c",
   parent directory, ./ in .gitmodules). Added workflow-markdown
   wiring assertions for all 4 dispatch sites + per-plan gate file
   extraction. Added quick.md guard tests: prompt injection assertion +
   behavioral fixture-repo tests that stage a submodule path and assert
   the guard exits non-zero with the ABORT message.

Test count: 5701 pass / 0 fail (was 5698/1 before).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-27 12:31:32 -04:00