diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 77aa0d3..624a768 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -27,8 +27,8 @@ - [ ] `pnpm test` passes -- [ ] `bash -n hook/pre-push` passes with no output -- [ ] `bash -n install.sh` passes with no output +- [ ] `pnpm run check:shell` passes with no output +- [ ] `pnpm run lint:shell` passes - [ ] Manually tested the hook on a real repository - [ ] Tested on macOS - [ ] Tested on Linux diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7686dd..a3189f9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,14 +26,17 @@ jobs: - name: Build TypeScript config layer run: pnpm build - - name: Test Node config layer + - name: Test Node layer and hook harness run: pnpm test - - name: Check hook syntax - run: bash -n hook/pre-push + - name: Check shell syntax + run: pnpm run check:shell - - name: Check installer syntax - run: bash -n install.sh + - name: Install ShellCheck + run: sudo apt-get update && sudo apt-get install --yes shellcheck + + - name: Check shell scripts with ShellCheck + run: pnpm run lint:shell - name: Verify hook is executable run: | diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7d45aaa..28b2568 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -89,18 +89,21 @@ commit as-is and customise from there. ## Testing your changes -Run the Node config tests before manual hook or installer checks: +Run the automated tests before manual hook or installer checks: ```bash # Install config parser dependencies pnpm install -# Typecheck the v2 config loader, then validate schema fixtures and templates +# Typecheck the Node layer, validate config fixtures, and run the hook harness +# against disposable Git repos and local tool/provider stubs pnpm test # Validate shell syntax -bash -n hook/pre-push -bash -n install.sh +pnpm run check:shell + +# Run ShellCheck's error-level static checks when ShellCheck is installed +pnpm run lint:shell # Test the installer locally (from inside a git repo) bash install.sh --template node @@ -118,8 +121,8 @@ verify the configured tools run correctly against changed files. ## Pull request checklist - [ ] `pnpm test` passes -- [ ] `bash -n hook/pre-push` passes with no output -- [ ] `bash -n install.sh` passes with no output +- [ ] `pnpm run check:shell` passes with no output +- [ ] `pnpm run lint:shell` passes when ShellCheck is installed - [ ] Commit messages follow Conventional Commits - [ ] New templates include all keys from an existing template - [ ] `README.md` updated if a new template was added diff --git a/docs/issue-3-hook-runner-test-harness-plan.md b/docs/issue-3-hook-runner-test-harness-plan.md new file mode 100644 index 0000000..bdc1dc8 --- /dev/null +++ b/docs/issue-3-hook-runner-test-harness-plan.md @@ -0,0 +1,215 @@ +# Issue 3 Hook And Runner Test Harness Plan + +This document narrows issue #3 into the knowledge gaps, open questions, and +execution plan for the behavioral harness that must exist before the installed +hook and Pushgate runner are rewritten. + +The broader product contract remains in `docs/product-contract-plan.md`. The +v2 config boundary frozen by issue #2 remains in +`docs/issue-2-config-schema-plan.md` and `docs/v2-config-schema.md`. + +## Known Context + +Issue #3 owns the first whole-workflow verification layer: + +1. Create temporary Git repositories in tests. +2. Invoke the current hook and later runner against controlled changes. +3. Stub deterministic tools and AI providers through `PATH`. +4. Assert exit codes, key terminal output, and observable artifacts. +5. Add shell syntax and static checks where practical. + +The repository is in a transition state that matters for the harness: + +| Area | Current state | Harness implication | +|---|---|---| +| Config parser | Node v2 `.pushgate.yml` loader and schema tests exist. | Reuse the existing Node test toolchain rather than creating a second test runtime without a reason. | +| Current hook | `hook/pre-push` still contains the legacy single Bash workflow and reads `.push-review.yml`. | Initial end-to-end coverage must distinguish characterization of this hook from v2 runner contract coverage. | +| Future runner | `pushgate pre-push` is the contract in docs, but issue #4 owns the thin hook and runner boundary. | The harness should make a future runner invocation easy without implementing that runner here. | +| Skip controls | Product docs define Git-config skip paths, but issue #18 owns implementation. | Skip scenarios need a place in the matrix now; AI-only and whole-runner skip assertions land when the controls exist. | +| Changed-file policy | Filenames with spaces, ignored paths, and deleted files are acceptance cases, while issue #5 owns final path semantics. | Test setup should create those repos now without freezing future path-policy internals. | +| AI providers | The current hook hard-codes `claude`; issue #10 owns the provider boundary. | The first provider stub can emulate the current CLI while the helper API stays provider-neutral. | +| CI | The Linux CI job already runs `pnpm test` for the Node config layer. | Issue #3 can extend that Linux path and should keep macOS-compatible helpers. | + +## Scope Boundaries + +Issue #3 should add test infrastructure and enough behavioral coverage to make +later rewrites observable. It should not redesign these backlog-owned surfaces: + +| Surface | Backlog owner | +|---|---| +| Thin installed hook plus `pushgate pre-push` runner | Issue #4 | +| Final changed-file and ignore-path semantics | Issue #5 | +| Deterministic command modes, timeouts, and richer runner behavior | Issue #6 | +| AI provider interface and Claude adapter | Issue #10 | +| Local AI mode guardrails | Issue #11 | +| Documented Git-config skip controls | Issue #18 | + +## Locked Definitions To Preserve + +- `git push` remains the primary developer entry point. +- The future installed `pre-push` hook delegates to `pushgate pre-push` and + returns its exit code. +- `.pushgate.yml` is the v2 config source. Legacy `.push-review.yml` behavior + is migration behavior, not the new public config vocabulary. +- V2 deterministic tool commands are argv arrays and `{changed_files}` is a + runner expansion token, not a shell interpolation contract. +- Local AI modes are `blocking`, `advisory`, and `off`, with `blocking` as the + default. +- Local hook behavior is not final enforcement because Git can bypass hooks. + +## Knowledge Gaps And Open Questions + +### Harness Boundary + +- Should the first end-to-end tests call `hook/pre-push` directly, install it + into `.git/hooks/pre-push` and execute a real push, or use both with one + smoke push and mostly direct invocation? +- Which current-hook behaviors are worth characterizing before issue #4, and + which assertions should wait for the v2 runner contract so legacy + `.push-review.yml` details are not made sticky? +- What stable harness API should future issue #4 tests use for runner stdin, + hook arguments, environment, and captured output? + +### Git Fixture Model + +- What minimal temporary-repo topology covers a target branch, a feature HEAD, + a bare remote for push smoke tests, deleted files, ignored paths, and + filenames with spaces without making every test expensive? +- Should branch resolution and remote-fetch behavior be exercised here or left + to issue #5 once its base-ref algorithm is frozen? +- Which Git identity and environment settings must be pinned so tests behave + the same on local macOS and Linux CI? + +### Stub Contract + +- How should stubs expose their invocations and generated artifacts to tests: + captured files in a harness-owned temp directory, stdout markers, or both? +- How much of the current `claude --print` output grammar should the first AI + stub model before the provider and structured-finding contracts exist? +- Should missing tool and missing provider cases be represented by absent + binaries on `PATH`, explicit failing stubs, or both? +- How should the harness prevent current hook paths such as update checks or + target-branch fetches from reaching the network? + +### Scenario Ownership + +- The issue requires pass, warning, blocking, and skipped outcomes. Which + skipped outcome is asserted before issue #18: Git's observable + `--no-verify` hook bypass, current hook early-exit paths, or only reserved + matrix rows for the later skip-control implementation? +- Should filenames with spaces, ignored paths, and deleted files assert only + that the workflow stays correct and safe now, or assert exact changed-file + lists before issue #5 owns the normalized list? +- Should provider failure preserve the current hook's allow-push behavior as a + characterization test, or should it be represented as an expected contract + change for later AI mode work? + +### Assertions And Portability + +- Which terminal lines are contract-level output worth asserting after ANSI + color is stripped, and which output is implementation noise? +- Should `ShellCheck` be a required local script, a Linux CI dependency, or an + optional check until the shell surface changes in issue #4? +- What is the supported shell and OS matrix beyond the issue requirement for a + Linux path and room for macOS coverage? + +## Working Decisions For Execution + +These defaults keep issue #3 actionable while leaving the backlog-owned +contracts open: + +1. Build the harness in the existing TypeScript `node:test` path. +2. Put reusable Git repo, command stub, environment isolation, and invocation + helpers under the test tree so later hook and runner tests can share them. +3. Invoke `hook/pre-push` directly for most current-hook characterization + tests. Add a real installed-hook push smoke test only when it proves hook + wiring or `--no-verify` behavior that direct invocation cannot prove. +4. Isolate `PATH`, `HOME`, Git author/committer identity, color-sensitive + output handling, and stub artifact directories per test. +5. Keep assertions focused on exit code, selected output markers, stub + invocation artifacts, and Git-observable results. Do not lock down the + legacy YAML parser shape or full terminal transcript. +6. Record scenario rows for v2 runner, AI-only skip, whole-runner skip, and + final changed-file normalization even when the implementation owner lands + later. + +## Initial Behavioral Matrix + +| Scenario | First harness target | Expected assertion | +|---|---|---| +| Tool pass plus AI pass | Current hook | Exit zero, tool stub called with changed file args, AI stub called, pass marker printed. | +| Tool failure | Current hook | Exit non-zero, AI stub not called, blocking marker printed. | +| AI warning result | Current hook | Exit zero, warning output visible, blocking count remains zero. | +| AI blocking result | Current hook | Exit non-zero and blocking summary visible. | +| Provider binary missing | Current hook | Exit non-zero with actionable missing-provider output. | +| Provider invocation failure | Current hook characterization | Current behavior captured without making later mode semantics implicit. | +| Filename with spaces | Current hook setup, future runner target | Stub artifact proves one logical path survives invocation. | +| Ignored changed path | Current hook setup, future path-policy target | Ignored path does not reach tool or AI artifacts for the asserted target. | +| Deleted changed path | Current hook setup, future path-policy target | Workflow does not try to read deleted content as a live file. | +| Hook bypass or scoped skip | Git smoke or reserved runner row | `--no-verify` can prove no hook invocation now; Git-config skip rows activate with issue #18. | + +## Execution Plan + +1. Add harness primitives to the existing test stack. + - Add test helpers for temporary directories, Git command execution, + deterministic Git identity, repo cleanup, and output capture. + - Add a temporary repo builder that creates a target branch and feature + change set without depending on the developer's global Git config. + - Add a `PATH` sandbox that can install executable tool, provider, and + network stubs while preserving the binaries needed by Git and Bash. + +2. Define invocation surfaces and stub artifacts. + - Add a current-hook invocation helper that runs the repository + `hook/pre-push` with isolated `HOME`, `PATH`, cwd, arguments, and stdin. + - Make stubs write JSON or line-oriented invocation artifacts inside the + test temp directory so assertions do not depend on exact transcript text. + - Stub `claude` responses for pass, warning, block, empty, and failing + outcomes, and stub configured deterministic tools for pass and fail. + - Block or stub network-relevant commands such as `curl` in harness + environments. + +3. Land current-hook characterization coverage. + - Cover pass, deterministic blocking, AI warning, AI blocking, missing + provider, and provider failure paths. + - Create fixture changes for a filename with spaces, an ignored path, and a + deleted file, then assert only the behavior owned by the current harness. + - Use focused output matchers that remove ANSI escapes before checking key + messages. + +4. Prepare the runner-facing extension point. + - Keep the repo builder and stub layer independent from the Bash hook. + - Add scenario names or table entries for future `pushgate pre-push` stdin + and argument coverage so issue #4 can swap in the runner without + rebuilding setup code. + - Keep `.pushgate.yml` fixtures available for v2 runner tests while any + legacy-hook fixtures stay clearly scoped to characterization. + +5. Add shell and Linux verification. + - Add scripts or test steps for `bash -n hook/pre-push` and + `bash -n install.sh`. + - Run error-level `ShellCheck` in Linux CI and keep the matching local + command available without hiding shell failures behind the Node tests. + - Extend the Linux CI workflow to run the harness plus shell checks. + +6. Document coverage and deferred rows. + - Update contributor-facing verification instructions when the harness + commands exist. + - Call out runner, skip-control, path-policy, and provider-contract rows + that are intentionally deferred to their owning issues. + +## Verification Target + +The issue is ready to close when: + +1. The harness creates isolated Git repos and stubs tool/provider calls without + live AI or network dependencies. +2. Automated coverage exercises pass, warning, blocking, missing-dependency, + filename-with-space, ignored-path, and deleted-file setup paths at the + scope owned by issue #3. +3. The skipped-outcome story is explicit: it is covered by an observable + current Git/hook case or represented by tests that activate with the + issue-18 controls. +4. Linux CI runs the harness and shell syntax checks, with helpers kept + compatible with local macOS runs. +5. The helper boundary is reusable when issue #4 introduces + `pushgate pre-push`. diff --git a/hook/pre-push b/hook/pre-push index b540372..05064f0 100755 --- a/hook/pre-push +++ b/hook/pre-push @@ -62,7 +62,7 @@ config_value() { config_list() { local key="$1" - awk "/^${key}:/{flag=1;next} flag && /^[^ ]/{flag=0} flag && /^\s*-/{gsub(/^\s*-\s*/,\"\"); print}" \ + awk "/^${key}:/{flag=1;next} flag && /^[^[:space:]]/{flag=0} flag && /^[[:space:]]*-/{gsub(/^[[:space:]]*-[[:space:]]*/,\"\"); print}" \ "$CONFIG_FILE" 2>/dev/null | tr -d '"' | tr -d "'" } @@ -111,8 +111,8 @@ while IFS= read -r f; do if [ -z "$pattern" ]; then continue fi + # shellcheck disable=SC2254 case "$f" in - # shellcheck disable=SC2254 $pattern) skip=true break @@ -505,4 +505,4 @@ else fi echo "" exit 0 -fi \ No newline at end of file +fi diff --git a/package.json b/package.json index 2b3792a..41a452f 100644 --- a/package.json +++ b/package.json @@ -8,8 +8,10 @@ }, "scripts": { "build": "tsc -p tsconfig.build.json", + "check:shell": "bash -n hook/pre-push && bash -n install.sh", + "lint:shell": "shellcheck --severity=error hook/pre-push install.sh", "typecheck": "tsc --noEmit", - "test": "pnpm run typecheck && tsx --test test/config.test.ts" + "test": "pnpm run typecheck && tsx --test test/*.test.ts" }, "dependencies": { "ajv": "^8.17.1", diff --git a/src/config/index.ts b/src/config/index.ts index 88ddfc6..b3c2cb2 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -34,8 +34,11 @@ const ajv = new Ajv({ allErrors: true, strict: true }); const validateSchema: ValidateFunction = ajv.compile(schema); +/** Base error shape thrown by the v2 config loader boundary. */ export class ConfigError extends Error { + /** Stable machine-readable error code for caller-specific rendering. */ readonly code: string; + /** Human-readable validation details when the error has diagnostics. */ readonly diagnostics: string[]; constructor(message: string, code: string, diagnostics: string[] = []) { @@ -46,7 +49,9 @@ export class ConfigError extends Error { } } +/** Raised when v2 YAML parses incorrectly or violates config validation. */ export class ConfigValidationError extends ConfigError { + /** Path used to identify the YAML source in diagnostics. */ readonly sourcePath: string; constructor(sourcePath: string, diagnostics: string[]) { @@ -61,7 +66,9 @@ export class ConfigValidationError extends ConfigError { } } +/** Raised when a repository has no v2 or legacy Pushgate config file. */ export class MissingConfigError extends ConfigError { + /** Expected `.pushgate.yml` path checked by the loader. */ readonly configPath: string; constructor(configPath: string) { @@ -73,8 +80,16 @@ export class MissingConfigError extends ConfigError { } } +/** + * Raised when only the legacy config exists. + * + * The loader does not parse `.push-review.yml` as v2 config; callers should + * surface this as migration guidance instead of silently adapting the file. + */ export class LegacyConfigError extends ConfigError { + /** Legacy `.push-review.yml` path found by the loader. */ readonly legacyPath: string; + /** Expected v2 `.pushgate.yml` path for migration output. */ readonly configPath: string; constructor(legacyPath: string, configPath: string) { @@ -87,7 +102,13 @@ export class LegacyConfigError extends ConfigError { } } -/** Parse, validate, and normalize a v2 Pushgate YAML config. */ +/** + * Parse, validate, and normalize a v2 Pushgate YAML config string. + * + * YAML syntax errors, schema errors, and active-AI provider selection errors + * are reported as `ConfigValidationError` before callers receive a normalized + * config object. + */ export function parseConfigYaml( source: string, sourcePath: string = CONFIG_FILENAME, @@ -121,8 +142,11 @@ export function parseConfigYaml( } /** - * Load the repository v2 config and surface legacy-file warnings separately - * from the normalized config value. + * Load the repository v2 config from disk. + * + * A present `.pushgate.yml` is parsed and returned with migration warnings for + * an accompanying legacy file. Legacy-only and missing-config repositories + * fail with dedicated errors so callers can choose actionable output. */ export async function loadConfig( repoRoot: string = process.cwd(), diff --git a/src/config/types.ts b/src/config/types.ts index 93f7a76..43c3102 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -1,26 +1,42 @@ +/** Local AI policy modes accepted by the v2 config boundary. */ export type AiMode = "blocking" | "advisory" | "off"; +/** Normalized diff-context settings consumed after v2 config validation. */ export interface ReviewConfig { + /** Local or remote-tracking branch name used as the review base. */ target_branch: string; + /** Surrounding diff lines included when review context is prepared. */ context_lines: number; + /** Diff-size cutoff below which later layers may include full file context. */ max_lines_for_full_file: number; } +/** Validated deterministic command config for the future runner. */ export interface ToolConfig { + /** Human-readable command label used in local output. */ name: string; + /** Argv tokens; `{changed_files}` remains a runner expansion token. */ command: string[]; + /** File extensions that scope changed-file execution when provided. */ extensions?: string[]; } +/** Provider-specific config extension block preserved for provider adapters. */ export type ProviderConfig = Record; +/** Normalized local AI selection and provider settings. */ export interface AiConfig { + /** Local AI behavior after config defaults are applied. */ mode: AiMode; + /** Provider selected for active AI modes. */ provider?: string; + /** Provider-specific settings keyed by provider identifier. */ providers: Record; } +/** Fully validated and defaulted v2 config returned to Pushgate consumers. */ export interface PushgateConfig { + /** Supported config schema version. */ version: 2; review: ReviewConfig; tools: ToolConfig[]; @@ -28,30 +44,42 @@ export interface PushgateConfig { ignore_paths: string[]; } +/** Parsed config plus repository file metadata exposed by `loadConfig`. */ export interface LoadedConfig { config: PushgateConfig; + /** Absolute path to the loaded `.pushgate.yml` file. */ path: string; + /** Non-fatal migration or compatibility messages for callers to surface. */ warnings: string[]; } +/** Raw review shape before optional v2 defaults are normalized. */ export interface RawReviewConfig { target_branch?: string; context_lines?: number; max_lines_for_full_file?: number; } +/** Raw deterministic command shape accepted after schema validation. */ export interface RawToolConfig { name: string; command: string[]; extensions?: string[]; } +/** Raw AI shape before default mode and provider diagnostics are applied. */ export interface RawAiConfig { mode?: AiMode; provider?: string; providers?: Record; } +/** + * Schema-validated v2 YAML shape before optional sections are normalized. + * + * AJV establishes this shape after parsing so normalization can fill stable + * defaults before later hook, runner, and AI layers read the config. + */ export interface RawPushgateConfig { version: 2; review?: RawReviewConfig; diff --git a/test/hook.test.ts b/test/hook.test.ts new file mode 100644 index 0000000..0f15829 --- /dev/null +++ b/test/hook.test.ts @@ -0,0 +1,224 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + cleanHookOutput, + createHookHarness, + type CommandResult, + type HookHarness, +} from "./support/hook-harness.js"; + +test("runs deterministic tool and AI stubs for a passing hook invocation", async () => { + await withHarness(async (harness) => { + await harness.writeLegacyConfig(legacyConfig()); + await harness.installToolStub(); + await harness.installClaudeStub(); + + const result = await harness.runHook(); + const output = cleanHookOutput(result); + + assert.equal(result.code, 0, output); + assert.match(output, /AI review passed/); + assert.deepEqual(await artifactLines(harness, "claude-args.txt"), ["--print"]); + + const toolArgs = await artifactLines(harness, "tool-args.txt"); + + assert.ok(toolArgs.includes("src/changed.ts")); + assert.ok(toolArgs.includes("src/deleted.ts")); + assert.ok(toolArgs.includes("src/file with spaces.ts")); + assert.ok(!toolArgs.includes("ignored/generated.ts")); + + const prompt = await requiredArtifact(harness, "claude-prompt.txt"); + + assert.match(prompt, /src\/file with spaces\.ts/); + assert.match(prompt, /src\/deleted\.ts/); + assert.doesNotMatch(prompt, /ignored\/generated\.ts/); + }); +}); + +test("blocks before AI when a deterministic tool fails", async () => { + await withHarness(async (harness) => { + await harness.writeLegacyConfig(legacyConfig()); + await harness.installToolStub(); + await harness.installClaudeStub(); + + const result = await harness.runHook({ + env: { PUSHGATE_TOOL_EXIT: "9" }, + }); + const output = cleanHookOutput(result); + + assert.equal(result.code, 1, output); + assert.match(output, /Tool record-changes failed/); + assert.equal(await harness.readArtifact("claude-args.txt"), null); + }); +}); + +test("blocks with a focused failure when a configured tool is missing", async () => { + await withHarness(async (harness) => { + await harness.writeLegacyConfig( + legacyConfig({ + command: "missing-tool {changed_files}", + name: "missing-tool", + }), + ); + await harness.installClaudeStub(); + + const result = await harness.runHook(); + const output = cleanHookOutput(result); + + assert.equal(result.code, 1, output); + assert.match(output, /Tool missing-tool failed/); + assert.equal(await harness.readArtifact("claude-args.txt"), null); + }); +}); + +test("allows a push through after an AI warning result", async () => { + await withHarness(async (harness) => { + await harness.writeLegacyConfig(legacyConfig(null)); + await harness.installClaudeStub(); + + const result = await harness.runHook({ + env: { PUSHGATE_CLAUDE_RESULT: "warning" }, + }); + const output = cleanHookOutput(result); + + assert.equal(result.code, 0, output); + assert.match(output, /WARNING/); + assert.match(output, /Blocking issues:\s+0/); + assert.match(output, /Warnings:\s+1/); + }); +}); + +test("blocks after an AI blocking result", async () => { + await withHarness(async (harness) => { + await harness.writeLegacyConfig(legacyConfig(null)); + await harness.installClaudeStub(); + + const result = await harness.runHook({ + env: { PUSHGATE_CLAUDE_RESULT: "block" }, + }); + const output = cleanHookOutput(result); + + assert.equal(result.code, 1, output); + assert.match(output, /Blocking issues:\s+1/); + assert.match(output, /Push blocked/); + }); +}); + +test("fails clearly when the current hook cannot find Claude", async () => { + await withHarness(async (harness) => { + await harness.writeLegacyConfig(legacyConfig(null)); + + const result = await harness.runHook(); + const output = cleanHookOutput(result); + + assert.equal(result.code, 1, output); + assert.match(output, /Claude Code CLI not found/); + assert.match(output, /Cannot perform AI review/); + }); +}); + +test("characterizes the current provider failure fallback", async () => { + await withHarness(async (harness) => { + await harness.writeLegacyConfig(legacyConfig(null)); + await harness.installClaudeStub(); + + const result = await harness.runHook({ + env: { PUSHGATE_CLAUDE_RESULT: "fail" }, + }); + const output = cleanHookOutput(result); + + assert.equal(result.code, 0, output); + assert.match(output, /Claude exited with code 7/); + assert.match(output, /allowing push to proceed/); + }); +}); + +test("proves git push --no-verify bypasses an installed pre-push hook", async () => { + await withHarness(async (harness) => { + await harness.writeLegacyConfig(legacyConfig()); + await harness.installToolStub(); + await harness.installClaudeStub(); + await harness.installInstalledHook(); + await harness.addBareOrigin(); + + const result = await harness.git([ + "push", + "--no-verify", + "origin", + "feature", + ]); + + assert.equal(result.code, 0, formatResult(result)); + assert.equal(await harness.readArtifact("tool-args.txt"), null); + assert.equal(await harness.readArtifact("claude-args.txt"), null); + }); +}); + +interface LegacyTool { + command: string; + name: string; +} + +function legacyConfig(tool: LegacyTool | null = defaultLegacyTool): string { + const lines = [ + "target_branch: main", + "context_lines: 3", + "max_lines_for_full_file: 1", + ]; + + if (tool) { + lines.push( + "tools:", + ` - name: ${tool.name}`, + ` command: ${tool.command}`, + ' extensions: [".ts"]', + ); + } + + lines.push("ignore_paths:", ' - "ignored/**"'); + + return `${lines.join("\n")}\n`; +} + +const defaultLegacyTool = { + command: "record-tool {changed_files}", + name: "record-changes", +}; + +async function withHarness( + callback: (harness: HookHarness) => Promise, +): Promise { + const harness = await createHookHarness(); + + try { + await callback(harness); + } finally { + await harness.cleanup(); + } +} + +async function artifactLines( + harness: HookHarness, + name: string, +): Promise { + return (await requiredArtifact(harness, name)).trimEnd().split("\n"); +} + +async function requiredArtifact( + harness: HookHarness, + name: string, +): Promise { + const artifact = await harness.readArtifact(name); + + assert.ok(artifact !== null, `Expected stub artifact ${name}.`); + return artifact; +} + +function formatResult(result: CommandResult): string { + return [ + `exit: ${String(result.code)}`, + `stdout:\n${result.stdout}`, + `stderr:\n${result.stderr}`, + ].join("\n"); +} diff --git a/test/support/hook-harness.ts b/test/support/hook-harness.ts new file mode 100644 index 0000000..c946c46 --- /dev/null +++ b/test/support/hook-harness.ts @@ -0,0 +1,466 @@ +import { spawn } from "node:child_process"; +import { + chmod, + copyFile, + mkdir, + mkdtemp, + readFile, + rm, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { delimiter, dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +/** Captured process result returned to harness tests instead of throwing. */ +export interface CommandResult { + /** Exit code from the child process, or `null` when a signal ended it. */ + code: number | null; + /** Signal that ended the child process, or `null` for normal completion. */ + signal: NodeJS.Signals | null; + /** Standard error captured as UTF-8 text. */ + stderr: string; + /** Standard output captured as UTF-8 text. */ + stdout: string; +} + +/** Overrides available when a test invokes Git or the repository hook. */ +export interface HookRunOptions { + /** Hook arguments to append after the hook path. */ + args?: string[]; + /** Environment overrides layered over the isolated harness environment. */ + env?: NodeJS.ProcessEnv; + /** Standard input delivered to the spawned process. */ + stdin?: string; +} + +/** + * Disposable Git workspace used to characterize hook and runner behavior. + * + * The harness owns one temp root with a seeded repository, an isolated home + * directory, executable stubs on `PATH`, and an artifact directory where those + * stubs record arguments and prompts for assertions. + */ +export interface HookHarness { + /** Directory where tool and provider stubs write assertion artifacts. */ + artifactsDir: string; + /** Directory prepended to `PATH` for test-local executables. */ + binDir: string; + /** Base isolated environment used for every harness command. */ + env: NodeJS.ProcessEnv; + /** Seeded feature repository used as the hook working directory. */ + repoRoot: string; + /** Parent directory containing every disposable harness resource. */ + tempRoot: string; + /** Create a local bare origin for tests that need a real `git push`. */ + addBareOrigin(): Promise; + /** Delete the full temporary harness tree. */ + cleanup(): Promise; + /** Run Git inside the seeded feature repository. */ + git(args: string[], options?: HookRunOptions): Promise; + /** Install the deterministic Claude CLI stub onto the sandbox `PATH`. */ + installClaudeStub(): Promise; + /** Copy the repository hook into `.git/hooks/pre-push` for push smoke tests. */ + installInstalledHook(): Promise; + /** Install a deterministic command stub under the given executable name. */ + installToolStub(name?: string): Promise; + /** Read a stub artifact, returning `null` when the stub did not create it. */ + readArtifact(name: string): Promise; + /** Run the repository hook directly without installing it into `.git`. */ + runHook(options?: HookRunOptions): Promise; + /** Write the legacy config consumed by the current Bash hook. */ + writeLegacyConfig(config: string): Promise; +} + +const hookSourcePath = fileURLToPath( + new URL("../../hook/pre-push", import.meta.url), +); + +const sandboxSystemPath = ["/usr/bin", "/bin", "/usr/sbin", "/sbin"]; + +/** + * Tool stub that records argv as one line per argument. + * + * Line-oriented artifacts preserve filenames with whitespace while keeping the + * test fixtures easy to inspect when a hook expectation fails. + */ +const toolStub = `#!/usr/bin/env bash +set -u + +printf '%s\n' "$@" > "$PUSHGATE_STUB_DIR/tool-args.txt" +printf 'tool invoked\n' >> "$PUSHGATE_STUB_DIR/tool-invocations.log" + +if [ -n "$PUSHGATE_TOOL_EXIT" ]; then + printf 'stub tool failed\n' >&2 + exit "$PUSHGATE_TOOL_EXIT" +fi + +printf 'stub tool passed\n' +`; + +/** + * Current-hook provider stub. + * + * The Bash hook still invokes `claude --print`, so this stub models only the + * response forms that the characterization tests need before provider adapters + * and structured output are moved behind the future runner boundary. + */ +const claudeStub = `#!/usr/bin/env bash +set -u + +printf '%s\n' "$@" > "$PUSHGATE_STUB_DIR/claude-args.txt" +cat > "$PUSHGATE_STUB_DIR/claude-prompt.txt" + +case "$PUSHGATE_CLAUDE_RESULT" in + pass) + printf '%s\n' \\ + 'SUMMARY' \\ + 'blocking_count: 0' \\ + 'warning_count: 0' \\ + 'verdict: PASS' + ;; + warning) + cat <<'EOF' +FINDING +category: test_coverage +severity: warning +file: src/changed.ts +line: 1 +message: stub warning +suggestion: keep the harness exercised + +SUMMARY +blocking_count: 0 +warning_count: 1 +verdict: PASS +EOF + ;; + block) + cat <<'EOF' +FINDING +category: security +severity: blocking +file: src/changed.ts +line: 1 +message: stub block +suggestion: fix the blocking finding + +SUMMARY +blocking_count: 1 +warning_count: 0 +verdict: BLOCK +EOF + ;; + fail) + printf 'stub provider failed\n' >&2 + exit 7 + ;; + empty) + ;; + *) + printf 'unknown claude stub result: %s\n' "$PUSHGATE_CLAUDE_RESULT" >&2 + exit 64 + ;; +esac +`; + +/** Non-network stub for the hook update check. */ +const curlStub = `#!/usr/bin/env bash +set -u + +printf 'curl blocked by hook harness\n' >> "$PUSHGATE_STUB_DIR/curl.log" +exit 22 +`; + +/** + * Create a fully isolated harness around a seeded feature repository. + * + * The repository starts with `main` at a baseline commit and `feature` at a + * second commit that changes a regular file, adds a filename with spaces, + * changes an ignorable path, and deletes a tracked file. Stubs are opt-in per + * test so missing-tool and missing-provider behavior can be asserted. + */ +export async function createHookHarness(): Promise { + const tempRoot = await mkdtemp(join(tmpdir(), "pushgate-hook-")); + const repoRoot = join(tempRoot, "repo"); + const homeDir = join(tempRoot, "home"); + const artifactsDir = join(tempRoot, "artifacts"); + const binDir = join(tempRoot, "bin"); + + await Promise.all( + [repoRoot, homeDir, artifactsDir, binDir].map((path) => + mkdir(path, { recursive: true }), + ), + ); + + const env = createSandboxEnv(homeDir, artifactsDir, binDir); + + await installExecutable(binDir, "curl", curlStub); + await seedFeatureRepo(repoRoot, env); + + return { + artifactsDir, + binDir, + env, + repoRoot, + tempRoot, + async addBareOrigin() { + const remoteRoot = join(tempRoot, "origin.git"); + + await checkedRun("git", ["init", "--quiet", "--bare", remoteRoot], { + cwd: tempRoot, + env, + }); + await checkedRun("git", ["remote", "add", "origin", remoteRoot], { + cwd: repoRoot, + env, + }); + + return remoteRoot; + }, + async cleanup() { + await rm(tempRoot, { force: true, recursive: true }); + }, + async git(args, options = {}) { + return runCommand("git", args, { + cwd: repoRoot, + env: { ...env, ...options.env }, + stdin: options.stdin, + }); + }, + async installClaudeStub() { + await installExecutable(binDir, "claude", claudeStub); + }, + async installInstalledHook() { + const installedHook = join(repoRoot, ".git", "hooks", "pre-push"); + + await copyFile(hookSourcePath, installedHook); + await chmod(installedHook, 0o755); + }, + async installToolStub(name = "record-tool") { + await installExecutable(binDir, name, toolStub); + }, + async readArtifact(name) { + try { + return await readFile(join(artifactsDir, name), "utf8"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") { + return null; + } + + throw error; + } + }, + async runHook(options = {}) { + return runCommand("bash", [hookSourcePath, ...(options.args ?? [])], { + cwd: repoRoot, + env: { ...env, ...options.env }, + stdin: options.stdin, + }); + }, + async writeLegacyConfig(config) { + await writeFile(join(repoRoot, ".push-review.yml"), config); + }, + }; +} + +/** + * Merge hook output streams and strip ANSI colors before matching messages. + * + * Hook tests use this for stable message assertions while artifact assertions + * cover exact tool/provider invocations. + */ +export function cleanHookOutput(result: CommandResult): string { + return `${result.stdout}\n${result.stderr}`.replace( + /\u001b\[[0-9;]*m/g, + "", + ); +} + +/** + * Seed the branch topology and changed-file shapes reused by hook scenarios. + */ +async function seedFeatureRepo( + repoRoot: string, + env: NodeJS.ProcessEnv, +): Promise { + await checkedRun("git", ["init", "--quiet", "--initial-branch=main"], { + cwd: repoRoot, + env, + }); + await checkedRun("git", ["config", "user.email", "hook-harness@example.test"], { + cwd: repoRoot, + env, + }); + await checkedRun("git", ["config", "user.name", "Pushgate Hook Harness"], { + cwd: repoRoot, + env, + }); + + await Promise.all([ + writeRepoFile(repoRoot, "src/changed.ts", "export const base = true;\n"), + writeRepoFile(repoRoot, "src/deleted.ts", "export const removeMe = true;\n"), + writeRepoFile( + repoRoot, + "ignored/generated.ts", + "export const generated = \"base\";\n", + ), + ]); + await commitAll(repoRoot, env, "baseline"); + + await checkedRun("git", ["switch", "--quiet", "-c", "feature"], { + cwd: repoRoot, + env, + }); + await Promise.all([ + writeRepoFile(repoRoot, "src/changed.ts", "export const changed = true;\n"), + writeRepoFile( + repoRoot, + "src/file with spaces.ts", + "export const spaced = true;\n", + ), + writeRepoFile( + repoRoot, + "ignored/generated.ts", + "export const generated = \"feature\";\n", + ), + rm(join(repoRoot, "src", "deleted.ts")), + ]); + await commitAll(repoRoot, env, "feature changes"); +} + +async function commitAll( + repoRoot: string, + env: NodeJS.ProcessEnv, + message: string, +): Promise { + await checkedRun("git", ["add", "--all"], { cwd: repoRoot, env }); + await checkedRun("git", ["commit", "--quiet", "-m", message], { + cwd: repoRoot, + env, + }); +} + +async function writeRepoFile( + repoRoot: string, + relativePath: string, + content: string, +): Promise { + const filePath = join(repoRoot, relativePath); + + await mkdir(dirname(filePath), { recursive: true }); + await writeFile(filePath, content); +} + +async function installExecutable( + binDir: string, + name: string, + content: string, +): Promise { + const executablePath = join(binDir, name); + + await writeFile(executablePath, content); + await chmod(executablePath, 0o755); +} + +/** + * Build the environment inherited by commands inside the disposable repo. + * + * User Git configuration and network update checks are isolated so tests do not + * depend on the developer machine, provider auth, or internet availability. + */ +function createSandboxEnv( + homeDir: string, + artifactsDir: string, + binDir: string, +): NodeJS.ProcessEnv { + return { + ...process.env, + GIT_CONFIG_NOSYSTEM: "1", + GIT_TERMINAL_PROMPT: "0", + HOME: homeDir, + LC_ALL: "C", + PATH: [binDir, ...sandboxSystemPath].join(delimiter), + PUSHGATE_CLAUDE_RESULT: "pass", + PUSHGATE_STUB_DIR: artifactsDir, + PUSHGATE_TOOL_EXIT: "", + TERM: "dumb", + XDG_CONFIG_HOME: join(homeDir, ".config"), + }; +} + +/** Run setup commands and fail early with captured output on non-zero exits. */ +async function checkedRun( + command: string, + args: string[], + options: CommandOptions, +): Promise { + const result = await runCommand(command, args, options); + + if (result.code !== 0) { + throw new Error( + [ + `${command} ${args.join(" ")} exited with ${String(result.code)}.`, + `stdout:\n${result.stdout}`, + `stderr:\n${result.stderr}`, + ].join("\n"), + ); + } +} + +interface CommandOptions { + cwd: string; + env: NodeJS.ProcessEnv; + stdin?: string; +} + +/** Spawn a command and capture its output without interpreting its exit code. */ +function runCommand( + command: string, + args: string[], + options: CommandOptions, +): Promise { + const stdinMode = options.stdin === undefined ? "ignore" : "pipe"; + + return new Promise((resolve, reject) => { + const child = spawn(command, args, { + cwd: options.cwd, + env: options.env, + stdio: [stdinMode, "pipe", "pipe"], + }); + let stderr = ""; + let stdout = ""; + + if (!child.stdout || !child.stderr) { + reject(new Error("Harness commands must capture stdout and stderr.")); + return; + } + + child.stdout.setEncoding("utf8"); + child.stderr.setEncoding("utf8"); + child.stdout.on("data", (data: string) => { + stdout += data; + }); + child.stderr.on("data", (data: string) => { + stderr += data; + }); + child.on("error", reject); + child.on("close", (code, signal) => { + resolve({ code, signal, stderr, stdout }); + }); + + if (options.stdin !== undefined) { + if (!child.stdin) { + reject(new Error("Harness command stdin was not piped.")); + return; + } + + child.stdin.on("error", (error: NodeJS.ErrnoException) => { + if (error.code !== "EPIPE") { + reject(error); + } + }); + child.stdin.end(options.stdin); + } + }); +}