From 0ac810fe64edcd26ef03d8c622c63d23502b9c19 Mon Sep 17 00:00:00 2001 From: Pushgate Hook Harness Date: Sat, 27 Jun 2026 11:48:35 -0300 Subject: [PATCH 1/5] Redesign Pushgate CLI output --- README.md | 7 +- bin/pushgate.mjs | 627 +++++++++++++++++++++----- hook/pre-push | 12 +- src/ai/transcript.ts | 102 +++-- src/cli.ts | 36 +- src/git/push.ts | 274 +++++++++++ src/runner/transcript.ts | 158 +++++-- src/terminal/format.ts | 211 +++++++++ src/version.ts | 1 + src/workflows/pre-push.ts | 108 ++++- src/workflows/terminal.ts | 6 +- src/workflows/warning-confirmation.ts | 6 +- test/ai.test.ts | 16 +- test/deterministic-runner.test.ts | 44 +- test/hook.test.ts | 14 +- test/runner.test.ts | 225 ++++++++- test/terminal.test.ts | 92 ++-- test/warning-confirmation.test.ts | 6 +- 18 files changed, 1632 insertions(+), 313 deletions(-) create mode 100644 src/terminal/format.ts create mode 100644 src/version.ts diff --git a/README.md b/README.md index 2e85fe7..a487fb2 100644 --- a/README.md +++ b/README.md @@ -220,7 +220,7 @@ git -c pushgate.skip-ai-check=true push git -c pushgate.skip-all-checks=true push ``` -The planned optional wrapper maps friendly flags to the same one-push config: +The optional wrapper maps friendly flags to the same one-push config: ```bash pushgate push --skip-ai-check @@ -239,7 +239,8 @@ This makes it possible to test an unpublished runner build in one repository without replacing the stable managed install for every repository on the machine. -Each hook run prints the resolved runner source and path, for example: +Routine hook runs keep runner wiring quiet. When diagnosing an override, set +`PUSHGATE_VERBOSE=1` to print the resolved runner source and path, for example: ```text [pushgate] Using runner from git config pushgate.runner: /absolute/path/to/bin/pushgate.mjs @@ -259,7 +260,7 @@ git config --unset --local pushgate.runner For one shell session or one command, use `PUSHGATE_RUNNER` instead: ```bash -PUSHGATE_RUNNER=/absolute/path/to/bin/pushgate.mjs git push +PUSHGATE_RUNNER=/absolute/path/to/bin/pushgate.mjs PUSHGATE_VERBOSE=1 git push ``` ## Updating diff --git a/bin/pushgate.mjs b/bin/pushgate.mjs index 30d1862..82e37b7 100755 --- a/bin/pushgate.mjs +++ b/bin/pushgate.mjs @@ -10069,6 +10069,176 @@ function runGitPush(args, options) { env: options.env }); } +async function resolveGitPushSuccessSummary(args, options) { + const parsed = parseGitPushArgs(args); + const currentUpstream = parsed.setsUpstream ? await readCurrentUpstream(options.env) : void 0; + const branch = parsed.branch ?? branchFromUpstream(currentUpstream) ?? await readCurrentBranch(options.env); + const remote = parsed.remote ?? remoteFromUpstream(currentUpstream) ?? (branch ? await readConfiguredBranchRemote(branch, options.env) : void 0); + const upstream = parsed.setsUpstream ? currentUpstream ?? upstreamFromParts(remote, branch) : void 0; + const remoteUrl = remote ? await readRemoteUrl(remote, options.env) : void 0; + return { + pullRequestUrl: remoteUrl && branch ? githubPullRequestUrl(remoteUrl, branch) : void 0, + upstream + }; +} +function formatGitPushSuccessSummary(summary) { + const rows = [" [ok] Branch pushed"]; + if (summary.upstream) { + rows.push(` [ok] Upstream set: ${summary.upstream}`); + } + let output = ` +Pushing branch +${rows.join("\n")} +`; + if (summary.pullRequestUrl) { + output += ` +Create a pull request: + ${summary.pullRequestUrl} +`; + } + return output; +} +function parseGitPushArgs(args) { + const positionals = []; + let parseOptions = true; + let setsUpstream = false; + let skipNext = false; + for (const arg of args) { + if (skipNext) { + skipNext = false; + continue; + } + if (parseOptions && arg === "--") { + parseOptions = false; + continue; + } + if (parseOptions && (arg === "-u" || arg === "--set-upstream")) { + setsUpstream = true; + continue; + } + if (parseOptions && arg.startsWith("-")) { + skipNext = optionTakesSeparateValue(arg); + continue; + } + positionals.push(arg); + } + return { + branch: positionals[1] ? branchFromRefspec(positionals[1]) : void 0, + remote: positionals[0], + setsUpstream + }; +} +function optionTakesSeparateValue(arg) { + return arg === "--exec" || arg === "--receive-pack" || arg === "--repo" || arg === "--push-option" || arg === "-o"; +} +function branchFromRefspec(refspec) { + let branch = refspec.trim(); + if (!branch || branch.includes("*")) { + return void 0; + } + if (branch.startsWith("+")) { + branch = branch.slice(1); + } + const remoteBranchSeparator = branch.lastIndexOf(":"); + if (remoteBranchSeparator >= 0) { + branch = branch.slice(remoteBranchSeparator + 1); + } + if (!branch || branch === "HEAD") { + return void 0; + } + if (branch.startsWith("refs/heads/")) { + return branch.slice("refs/heads/".length); + } + if (branch.startsWith("refs/")) { + return void 0; + } + return branch; +} +async function readCurrentBranch(env) { + const branch = await readGitStdout(["rev-parse", "--abbrev-ref", "HEAD"], env); + if (!branch || branch === "HEAD") { + return void 0; + } + return branch; +} +async function readCurrentUpstream(env) { + return readGitStdout( + ["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}"], + env + ); +} +async function readConfiguredBranchRemote(branch, env) { + return readGitStdout(["config", "--get", `branch.${branch}.remote`], env); +} +async function readRemoteUrl(remote, env) { + return readGitStdout(["remote", "get-url", remote], env); +} +async function readGitStdout(args, env) { + try { + const result = await runCommand({ + args, + command: "git", + env + }); + if (result.code !== 0) { + return void 0; + } + const output = result.stdout.trim(); + return output ? output : void 0; + } catch { + return void 0; + } +} +function upstreamFromParts(remote, branch) { + return remote && branch ? `${remote}/${branch}` : void 0; +} +function remoteFromUpstream(upstream) { + const separator = upstream?.indexOf("/") ?? -1; + return separator > 0 ? upstream?.slice(0, separator) : void 0; +} +function branchFromUpstream(upstream) { + const separator = upstream?.indexOf("/") ?? -1; + return separator > 0 ? upstream?.slice(separator + 1) : void 0; +} +function githubPullRequestUrl(remoteUrl, branch) { + const repository = githubRepository(remoteUrl); + if (!repository) { + return void 0; + } + return `https://github.com/${repository.owner}/${repository.repo}/pull/new/${encodeURIComponent( + branch + )}`; +} +function githubRepository(remoteUrl) { + const trimmed = remoteUrl.trim(); + const sshMatch = /^git@github\.com:([^/\s]+)\/([^/\s]+?)(?:\.git)?$/i.exec( + trimmed + ); + if (sshMatch) { + return { + owner: sshMatch[1], + repo: sshMatch[2] + }; + } + try { + const parsed = new URL(trimmed); + if (parsed.protocol !== "https:" || parsed.hostname.toLowerCase() !== "github.com") { + return void 0; + } + const pathParts = parsed.pathname.replace(/^\/|\/$/g, "").split("/"); + if (pathParts.length !== 2) { + return void 0; + } + const [owner, repoWithPossibleSuffix] = pathParts; + const repo = repoWithPossibleSuffix.endsWith(".git") ? repoWithPossibleSuffix.slice(0, -".git".length) : repoWithPossibleSuffix; + return owner && repo ? { owner, repo } : void 0; + } catch { + return void 0; + } +} + +// src/workflows/pre-push.ts +import { basename } from "node:path"; // src/ai/guardrails.ts function evaluateChangedFileGuardrails(options) { @@ -26513,6 +26683,136 @@ function countTextLines(text) { return text.endsWith("\n") ? newlineCount : newlineCount + 1; } +// src/terminal/format.ts +var ANSI = { + blue: ["\x1B[34m", "\x1B[39m"], + bold: ["\x1B[1m", "\x1B[22m"], + dim: ["\x1B[2m", "\x1B[22m"], + green: ["\x1B[32m", "\x1B[39m"], + red: ["\x1B[31m", "\x1B[39m"], + yellow: ["\x1B[33m", "\x1B[39m"] +}; +var LABEL_WIDTH = 18; +function writeHeader(stream, lines) { + for (const line of lines) { + writeLine(stream, line); + } + if (lines.length > 0) { + writeLine(stream, ""); + } +} +function writeSection(stream, title, options = {}) { + writeLine(stream, style(title, "bold", withStream(stream, options))); +} +function writeResultRow(stream, status, label, detail, options = {}) { + const styleOptions = withStream(stream, options); + const symbol2 = statusSymbol(status, styleOptions); + const styledSymbol = styleStatus(symbol2, status, styleOptions); + const paddedLabel = label.padEnd(LABEL_WIDTH, " "); + const suffix = detail ? ` ${detail}` : ""; + writeLine(stream, ` ${styledSymbol} ${paddedLabel}${suffix}`.trimEnd()); +} +function writeDetail(stream, detail) { + writeLine(stream, ` ${detail}`); +} +function writeIndentedBlock(stream, lines) { + for (const line of lines) { + writeLine(stream, ` ${line}`); + } +} +function writeLine(stream, line = "") { + stream.write(`${line} +`); +} +function formatCount(count, singular, plural = `${singular}s`) { + return `${String(count)} ${count === 1 ? singular : plural}`; +} +function humanizeIdentifier(value) { + const stripped = value.replace(/^(policy|plugin):/, ""); + const words = stripped.replace(/[_-]+/g, " ").trim().split(/\s+/).filter(Boolean); + if (words.length === 0) { + return value; + } + return words.map( + (word, index) => index === 0 ? capitalize(word) : word.toLowerCase() + ).join(" "); +} +function capitalize(value) { + if (value.length === 0) { + return value; + } + return `${value[0]?.toUpperCase() ?? ""}${value.slice(1)}`; +} +function statusSymbol(status, options) { + const unicode = supportsUnicode(options); + if (!unicode) { + return { + blocked: "[block]", + info: "[info]", + passed: "[ok]", + skipped: "[skip]", + warning: "[warn]" + }[status]; + } + return { + blocked: "x", + info: "i", + passed: "\u2713", + skipped: "-", + warning: "!" + }[status]; +} +function styleStatus(value, status, options) { + switch (status) { + case "blocked": + return style(value, "red", options); + case "info": + return style(value, "blue", options); + case "passed": + return style(value, "green", options); + case "skipped": + return style(value, "dim", options); + case "warning": + return style(value, "yellow", options); + } +} +function style(value, color, options) { + if (!supportsColor(options)) { + return value; + } + const [open, close] = ANSI[color]; + return `${open}${value}${close}`; +} +function supportsColor(options) { + const env = options.env ?? process.env; + if (env.NO_COLOR !== void 0 || env.NODE_DISABLE_COLORS !== void 0) { + return false; + } + if (env.FORCE_COLOR !== void 0 && env.FORCE_COLOR !== "0") { + return true; + } + return options.stream?.isTTY === true; +} +function supportsUnicode(options) { + const env = options.env ?? process.env; + if (env.PUSHGATE_ASCII === "1") { + return false; + } + if (options.stream?.isTTY !== true) { + return false; + } + if (process.platform !== "win32") { + return env.TERM !== "linux"; + } + return Boolean(env.WT_SESSION || env.TERMINUS_SUBLIME || env.CI); +} +function withStream(stream, options) { + return { + ...options, + stream: options.stream ?? stream + }; +} + // src/ai/transcript.ts function renderLocalAiTranscript(events, stdout) { for (const event of events) { @@ -26522,94 +26822,96 @@ function renderLocalAiTranscript(events, stdout) { function renderLocalAiTranscriptEvent(event, stdout) { switch (event.kind) { case "skip-no-files": - writeLine(stdout, "[pushgate] No changed files to review with local AI."); + writeResultRow(stdout, "skipped", "No changed files to review"); return; case "block-changed-lines": - writeLine( + writeResultRow( stdout, - `[pushgate] BLOCK local AI because ${String(event.changedLineCount)} changed line(s) exceed ai.max_changed_lines ${String(event.maxChangedLines)}.` + "blocked", + "Changed lines", + `${String(event.changedLineCount)} changed lines exceed ai.max_changed_lines ${String(event.maxChangedLines)}` ); return; case "skip-prompt-tokens": - writeLine( + writeResultRow( stdout, - `[pushgate] Skipping local AI because the rendered prompt is approximately ${String(event.estimatedPromptTokens)} token(s), exceeding ai.max_prompt_tokens ${String(event.maxPromptTokens)}.` + "skipped", + "Prompt budget", + `approximately ${String(event.estimatedPromptTokens)} tokens exceeds ai.max_prompt_tokens ${String(event.maxPromptTokens)}` ); return; case "review-start": - writeLine( - stdout, - `[pushgate] Running local AI review with ${event.providerId} on ${String(event.changedFileCount)} changed file(s).` - ); + writeDetail(stdout, `Provider: ${capitalize(event.providerId)}`); + writeDetail(stdout, `Files reviewed: ${String(event.changedFileCount)}`); return; case "full-file-context": - writeLine( + writeDetail( stdout, - `[pushgate] Local AI prompt includes ${String(event.diffLineCount)} diff line(s) plus ${String(event.fullFileCount)} full file(s) for extra context.` + `Context: ${formatCount(event.diffLineCount, "diff line")} plus ${formatCount(event.fullFileCount, "full file")} for extra context` ); return; case "provider-failure": { - const label = event.aiMode === "advisory" ? "WARN" : "BLOCK"; - writeLine( + const status = event.aiMode === "advisory" ? "warning" : "blocked"; + writeResultRow( stdout, - `[pushgate] ${label} local AI provider ${event.result.provider} failed: ${event.result.message}` + status, + `${capitalize(event.result.provider)} provider`, + event.result.message ); if (event.result.detail) { - for (const line of event.result.detail.split("\n")) { - writeLine(stdout, `[pushgate] Detail: ${line}`); - } + writeDetail(stdout, "Detail:"); + writeIndentedBlock(stdout, event.result.detail.split("\n")); } if (event.result.output) { - writeLine(stdout, "[pushgate] Provider output:"); - for (const line of event.result.output.split("\n")) { - writeLine(stdout, `[pushgate] ${line}`); - } + writeDetail(stdout, "Provider output:"); + writeIndentedBlock(stdout, event.result.output.split("\n")); } return; } case "normalization-note": - writeLine(stdout, `[pushgate] Note: ${event.note}`); + writeDetail(stdout, `Note: ${event.note}`); return; case "review-passed": - writeLine(stdout, "[pushgate] Local AI review passed with no findings."); + writeResultRow(stdout, "passed", "No findings"); return; case "finding": { - const label = event.finding.severity === "blocking" ? "BLOCK" : "WARN"; + const status = event.finding.severity === "blocking" ? "blocked" : "warning"; const location = event.finding.line === "N/A" ? event.finding.file : `${event.finding.file}:${event.finding.line}`; - writeLine( + writeResultRow( stdout, - `[pushgate] ${label} AI ${event.finding.category} at ${location}.` + status, + `AI ${humanizeCategory(event.finding.category)}`, + location ); - writeLine(stdout, `[pushgate] Message: ${event.finding.message}`); - writeLine(stdout, `[pushgate] Suggestion: ${event.finding.suggestion}`); + writeDetail(stdout, `Message: ${event.finding.message}`); + writeDetail(stdout, `Suggestion: ${event.finding.suggestion}`); return; } case "review-summary": - writeLine( - stdout, - `[pushgate] Local AI review finished: ${String(event.summary.blockingCount)} blocking finding(s), ${String(event.summary.warningCount)} warning(s).` - ); + if (event.summary.blockingCount > 0 || event.summary.warningCount > 0) { + writeDetail( + stdout, + `Finished with ${formatCount(event.summary.blockingCount, "blocking finding")} and ${formatCount(event.summary.warningCount, "warning")}.` + ); + } return; case "advisory-continue": - writeLine(stdout, "[pushgate] Continuing because ai.mode is advisory."); + writeDetail(stdout, "Continuing because ai.mode is advisory."); return; case "provider-blocked": - writeLine( - stdout, - "[pushgate] Local AI is blocking in this repository. Fix the provider issue or use git -c pushgate.skip-ai-check=true push to bypass only the AI phase for one push." - ); + writeLine(stdout); + writeLine(stdout, "Local AI is blocking in this repository."); + writeLine(stdout, "Fix the provider issue, or use `git -c pushgate.skip-ai-check=true push` to bypass only the AI phase for one push."); return; case "review-blocked": - writeLine( - stdout, - "[pushgate] Local AI review blocked the push. Fix the findings above or use git -c pushgate.skip-ai-check=true push to bypass only the AI phase for one push." - ); + writeLine(stdout); + writeLine(stdout, "Local AI review blocked the push."); + writeLine(stdout, "Fix the findings above, or use `git -c pushgate.skip-ai-check=true push` to bypass only the AI phase for one push."); return; } } -function writeLine(stream, line) { - stream.write(`${line} -`); +function humanizeCategory(category) { + return category.replace(/_/g, " "); } // src/ai/verdict.ts @@ -27042,78 +27344,117 @@ function summarizeDeterministicResults(results) { // src/runner/transcript.ts function createDeterministicTranscript(stdout) { + const warnings = []; + const blockers = []; return { writeFailFast() { - writeLine2( - stdout, - "[pushgate] Stopping deterministic checks after blocking failure because fail_fast is true." - ); + writeDetail(stdout, "Stopped after a blocking failure because fail_fast is true."); }, writeNoChecks() { - writeLine2(stdout, "[pushgate] No deterministic checks configured."); + writeSection(stdout, "Checks"); + writeResultRow(stdout, "skipped", "No checks configured"); + writeLine(stdout); }, writePolicyResult(result) { - const labelByStatus = { - blocked: "BLOCK", - passed: "PASS", - warning: "WARN" - }; - const detail = result.detail ? `: ${result.detail}` : ""; - writeLine2( - stdout, - `[pushgate] ${labelByStatus[result.status]} ${result.name}${detail}.` - ); + writeCheckResult(result.name, result); }, writePluginResult(name, result) { - writeRunnableResult(name, result); + writeCheckResult(name, result); }, writeStart(checkCount) { - writeLine2( - stdout, - `[pushgate] Running ${String(checkCount)} deterministic check(s).` - ); + writeSection(stdout, "Checks"); + writeDetail(stdout, `Running ${formatCount(checkCount, "check")}.`); }, writeSummary(summary) { - writeLine2( - stdout, - `[pushgate] Deterministic checks finished: ${String(summary.blockedCount)} blocking failure(s), ${String(summary.warningCount)} warning(s).` - ); + writeLine(stdout); if (summary.blockedCount > 0) { - writeLine2( + writeLine( stdout, - "[pushgate] Fix the blocking command failures before pushing, or use git push --no-verify to bypass local hooks intentionally." + `Checks completed with ${formatCount(summary.blockedCount, "blocking failure")} and ${formatCount(summary.warningCount, "warning")}.` ); + writeLine(stdout); + writeSection(stdout, summary.blockedCount === 1 ? "Blocked" : "Blocked checks"); + for (const blocker of blockers) { + writeDetail(stdout, `${blocker} failed and is configured as a blocking check.`); + } + writeLine(stdout); + writeLine( + stdout, + "Fix the blocking failures above, or use `git push --no-verify` only when you intend to bypass local hooks." + ); + return; } + if (summary.warningCount > 0) { + writeLine( + stdout, + `Checks completed with ${formatCount(summary.warningCount, "non-blocking warning")}.` + ); + writeLine(stdout); + writeSection(stdout, summary.warningCount === 1 ? "Warning" : "Warnings"); + for (const warning of warnings) { + writeDetail(stdout, `${warning} failed, but this check does not block the push.`); + } + writeLine(stdout); + return; + } + writeLine(stdout, "Checks passed."); + writeLine(stdout); }, writeToolResult(tool, result) { - writeRunnableResult(tool.name, result); + writeCheckResult(tool.name, result); } }; - function writeRunnableResult(name, result) { - if (result.status === "passed") { - writeLine2(stdout, `[pushgate] PASS ${name}.`); - return; + function writeCheckResult(name, result) { + const display = displayCheck(name); + const detail = formatDetail(name, result.detail); + const status = mapStatus(result.status); + writeResultRow(stdout, status, display.label, detail ?? display.detail); + if (result.outputTail) { + writeDetail(stdout, "Command output:"); + writeIndentedBlock(stdout, result.outputTail.split("\n")); } - if (result.status === "skipped") { - writeLine2(stdout, `[pushgate] SKIP ${name}: ${result.detail}.`); - return; + if (result.status === "warning") { + warnings.push(display.label); } - const label = result.status === "warning" ? "WARN" : "BLOCK"; - writeLine2( - stdout, - `[pushgate] ${label} ${name}: ${result.detail ?? "command failed"}.` + if (result.status === "blocked") { + blockers.push(display.label); + } + } +} +function displayCheck(name) { + if (name === "policy:diff_size") { + return { label: "Diff size" }; + } + if (name === "policy:forbidden_paths") { + return { label: "Forbidden paths" }; + } + if (name === "plugin:gitleaks") { + return { detail: "gitleaks", label: "Secrets scan" }; + } + return { label: humanizeIdentifier(name) }; +} +function formatDetail(name, detail) { + if (!detail) { + return void 0; + } + if (name === "policy:diff_size") { + const passed = detail.match( + /^(\d+) changed line\(s\) within max_changed_lines (\d+)$/ ); - if (result.outputTail) { - writeLine2(stdout, "[pushgate] Command output:"); - for (const line of result.outputTail.split("\n")) { - writeLine2(stdout, `[pushgate] ${line}`); - } + if (passed) { + return `${passed[1]} / ${passed[2]} changed lines`; } } + return detail; } -function writeLine2(stream, line) { - stream.write(`${line} -`); +function mapStatus(status) { + const statusByResult = { + blocked: "blocked", + passed: "passed", + skipped: "skipped", + warning: "warning" + }; + return statusByResult[status]; } // src/runner/tool-command.ts @@ -27274,6 +27615,9 @@ function requireChangedFileResolution(changedFileResolution) { ); } +// src/version.ts +var PUSHGATE_VERSION = "3.5.0"; + // src/workflows/run-decisions.ts function buildPrePushConfigDecision(skipControls) { if (skipControls.active.kind === "skip-all-checks") { @@ -27389,7 +27733,7 @@ function confirmWithInteractiveTerminal(question) { } writeSync( terminal.outputFd, - "[pushgate] Please answer yes(y) or no(n).\n" + "Please answer `y` or `n`.\n" ); } } catch (error51) { @@ -27402,14 +27746,14 @@ function confirmWithInteractiveTerminal(question) { } } function formatYesNoPrompt(question) { - return `${question} yes(y) / no(n) `; + return `${question} [y/N] `; } function normalizeAnswer(answer) { const normalized = answer.trim().toLowerCase(); if (normalized === "y" || normalized === "yes") { return "yes"; } - if (normalized === "n" || normalized === "no") { + if (normalized === "" || normalized === "n" || normalized === "no") { return "no"; } return "invalid"; @@ -27526,7 +27870,7 @@ function createTerminalWarningConfirmer(options = {}) { const terminal = options.terminal ?? createInteractiveTerminal(); return async (request) => { try { - return terminal.confirm(formatWarningQuestion(request)); + return terminal.confirm("Continue with push?"); } catch (error51) { if (error51 instanceof InteractiveTerminalError) { throw new WarningConfirmationError( @@ -27537,14 +27881,15 @@ function createTerminalWarningConfirmer(options = {}) { } }; } -function formatWarningQuestion(request) { - return `[pushgate] ${request.phase} produced ${String(request.warningCount)} warning(s). Continue with warnings?`; -} // src/workflows/pre-push.ts async function runPrePushWorkflow(io) { - await drainStdin(io.stdin); + const hookContext = buildPrePushContext({ + args: io.hookArgs ?? [], + stdin: await readStdin(io.stdin) + }); const repoRoot = await resolveGitRepositoryRoot(io.env); + writePrePushHeader(io.stdout, repoRoot, hookContext); const skipControls = await resolveSkipControlState(repoRoot, io.env); const configDecision = buildPrePushConfigDecision(skipControls); if (configDecision.kind === "skip") { @@ -27602,13 +27947,20 @@ async function runPrePushWorkflow(io) { })) { return 1; } + writeLine(io.stdout); + writeLine(io.stdout, "Pushgate passed. Git is pushing..."); return 0; } async function runLocalAiPhase(config2, decision, changedFileResolution, options) { if (decision.kind === "skip") { - writeVisibleSkipReason(options.stdout, decision.reason); + const message = formatRunSkipReason(decision.reason); + if (message !== null) { + writeSection(options.stdout, "AI review"); + writeResultRow(options.stdout, "skipped", message); + } return { exitCode: 0, warningCount: 0 }; } + writeSection(options.stdout, "AI review"); return await runLocalAiReview({ aiConfig: config2.ai, changedFileResolution: requireChangedFileResolution2( @@ -27633,22 +27985,22 @@ async function confirmWarningsBeforeContinuing(options) { }); if (confirmed) { options.stdout.write( - `[pushgate] Continuing with ${String(options.warningCount)} warning(s) from ${options.phase} after confirmation. + `Continuing with ${String(options.warningCount)} warning(s) from ${options.phase} after confirmation. ` ); return true; } options.stdout.write( - `[pushgate] Push blocked because ${options.phase} produced ${String(options.warningCount)} warning(s) and continuation was not confirmed. + `Push blocked because ${options.phase} produced ${String(options.warningCount)} warning(s) and continuation was not confirmed. ` ); return false; } catch (error51) { if (error51 instanceof WarningConfirmationError) { - options.stdout.write(`[pushgate] ${error51.message} + options.stdout.write(`${error51.message} `); options.stdout.write( - "[pushgate] Push blocked because warning confirmation could not be collected.\n" + "Push blocked because warning confirmation could not be collected.\n" ); return false; } @@ -27668,8 +28020,7 @@ async function maybeResolveChangedFiles(config2, options) { function writeVisibleSkipReason(stdout, reason) { const message = formatRunSkipReason(reason); if (message !== null) { - stdout.write(`[pushgate] ${message} -`); + writeResultRow(stdout, "skipped", message); } } function requireChangedFileResolution2(changedFileResolution, phaseName) { @@ -27680,14 +28031,53 @@ function requireChangedFileResolution2(changedFileResolution, phaseName) { `Pushgate could not prepare changed files for the ${phaseName}.` ); } -function drainStdin(stdin) { +function writePrePushHeader(stdout, repoRoot, context) { + const lines = [ + `Pushgate v${PUSHGATE_VERSION} - pre-push`, + `Repo: ${basename(repoRoot)}` + ]; + if (context.branch) { + lines.push(`Branch: ${context.branch}`); + } + if (context.remote) { + lines.push(`Remote: ${context.remote}`); + } + writeHeader(stdout, lines); +} +function buildPrePushContext(options) { + return { + branch: parseBranchFromPrePushInput(options.stdin), + remote: options.args[0] + }; +} +function parseBranchFromPrePushInput(input) { + for (const line of input.split(/\r?\n/)) { + const trimmed = line.trim(); + if (!trimmed) { + continue; + } + const [localRef] = trimmed.split(/\s+/, 1); + if (localRef?.startsWith("refs/heads/")) { + return localRef.slice("refs/heads/".length); + } + } + return void 0; +} +function readStdin(stdin) { return new Promise((resolve, reject) => { if (stdin.isTTY) { - resolve(); + resolve(""); return; } + let input = ""; + stdin.setEncoding("utf8"); stdin.on("error", reject); - stdin.on("end", resolve); + stdin.on("data", (chunk) => { + input += chunk; + }); + stdin.on("end", () => { + resolve(input); + }); stdin.resume(); }); } @@ -27718,7 +28108,7 @@ async function main(argv = process.argv.slice(2), io = { `); return 0; case "pre-push": - return runPrePushCommand(io); + return runPrePushCommand(args, io); case "push": return runPushCommand(args, io); default: @@ -27729,9 +28119,9 @@ async function main(argv = process.argv.slice(2), io = { return 64; } } -async function runPrePushCommand(io) { +async function runPrePushCommand(args, io) { try { - return await runPrePushWorkflow(io); + return await runPrePushWorkflow({ ...io, hookArgs: args }); } catch (error51) { writePushgateError(io.stderr, error51); return 1; @@ -27750,6 +28140,9 @@ async function runPushCommand(args, io) { ); }); if (result.code !== null) { + if (result.code === 0) { + await writeGitPushSuccessSummary(parsed.gitPushArgs, io); + } return result.code; } throw new SkipControlError( @@ -27760,6 +28153,18 @@ async function runPushCommand(args, io) { return 1; } } +async function writeGitPushSuccessSummary(gitPushArgs, io) { + try { + io.stdout.write( + formatGitPushSuccessSummary( + await resolveGitPushSuccessSummary(gitPushArgs, { + env: io.env + }) + ) + ); + } catch { + } +} function writeUsageError(stderr, message) { stderr.write(`${message} diff --git a/hook/pre-push b/hook/pre-push index f48554d..a2eea1c 100755 --- a/hook/pre-push +++ b/hook/pre-push @@ -30,6 +30,12 @@ info() { printf '[pushgate] %s\n' "$*" } +verbose() { + if [ "${PUSHGATE_VERBOSE:-}" = "1" ]; then + info "$@" + fi +} + error() { printf '[pushgate] %s\n' "$*" >&2 } @@ -121,7 +127,7 @@ if [ "$RUNNER_PROTOCOL" != "$HOOK_PROTOCOL" ]; then exit 1 fi -info "Pushgate pre-push hook v${HOOK_VERSION} with protocol ${HOOK_PROTOCOL}" -info "Repository root: ${REPO_ROOT}" -info "Using runner from ${PUSHGATE_RUNNER_SOURCE}: ${PUSHGATE_RUNNER}" +verbose "Pushgate pre-push hook v${HOOK_VERSION} with protocol ${HOOK_PROTOCOL}" +verbose "Repository root: ${REPO_ROOT}" +verbose "Using runner from ${PUSHGATE_RUNNER_SOURCE}: ${PUSHGATE_RUNNER}" exec "$PUSHGATE_RUNNER" pre-push "$@" diff --git a/src/ai/transcript.ts b/src/ai/transcript.ts index 4afc06e..4f89dfa 100644 --- a/src/ai/transcript.ts +++ b/src/ai/transcript.ts @@ -1,3 +1,11 @@ +import { + capitalize, + formatCount, + writeDetail, + writeIndentedBlock, + writeLine, + writeResultRow, +} from "../terminal/format.js"; import type { LocalAiTranscriptEvent } from "./types.js"; export function renderLocalAiTranscript( @@ -15,101 +23,107 @@ function renderLocalAiTranscriptEvent( ): void { switch (event.kind) { case "skip-no-files": - writeLine(stdout, "[pushgate] No changed files to review with local AI."); + writeResultRow(stdout, "skipped", "No changed files to review"); return; case "block-changed-lines": - writeLine( + writeResultRow( stdout, - `[pushgate] BLOCK local AI because ${String(event.changedLineCount)} changed line(s) exceed ai.max_changed_lines ${String(event.maxChangedLines)}.`, + "blocked", + "Changed lines", + `${String(event.changedLineCount)} changed lines exceed ai.max_changed_lines ${String(event.maxChangedLines)}`, ); return; case "skip-prompt-tokens": - writeLine( + writeResultRow( stdout, - `[pushgate] Skipping local AI because the rendered prompt is approximately ${String(event.estimatedPromptTokens)} token(s), exceeding ai.max_prompt_tokens ${String(event.maxPromptTokens)}.`, + "skipped", + "Prompt budget", + `approximately ${String(event.estimatedPromptTokens)} tokens exceeds ai.max_prompt_tokens ${String(event.maxPromptTokens)}`, ); return; case "review-start": - writeLine( - stdout, - `[pushgate] Running local AI review with ${event.providerId} on ${String(event.changedFileCount)} changed file(s).`, - ); + writeDetail(stdout, `Provider: ${capitalize(event.providerId)}`); + writeDetail(stdout, `Files reviewed: ${String(event.changedFileCount)}`); return; case "full-file-context": - writeLine( + writeDetail( stdout, - `[pushgate] Local AI prompt includes ${String(event.diffLineCount)} diff line(s) plus ${String(event.fullFileCount)} full file(s) for extra context.`, + `Context: ${formatCount(event.diffLineCount, "diff line")} plus ${formatCount(event.fullFileCount, "full file")} for extra context`, ); return; case "provider-failure": { - const label = event.aiMode === "advisory" ? "WARN" : "BLOCK"; + const status = event.aiMode === "advisory" ? "warning" : "blocked"; - writeLine( + writeResultRow( stdout, - `[pushgate] ${label} local AI provider ${event.result.provider} failed: ${event.result.message}`, + status, + `${capitalize(event.result.provider)} provider`, + event.result.message, ); if (event.result.detail) { - for (const line of event.result.detail.split("\n")) { - writeLine(stdout, `[pushgate] Detail: ${line}`); - } + writeDetail(stdout, "Detail:"); + writeIndentedBlock(stdout, event.result.detail.split("\n")); } if (event.result.output) { - writeLine(stdout, "[pushgate] Provider output:"); - - for (const line of event.result.output.split("\n")) { - writeLine(stdout, `[pushgate] ${line}`); - } + writeDetail(stdout, "Provider output:"); + writeIndentedBlock(stdout, event.result.output.split("\n")); } return; } case "normalization-note": - writeLine(stdout, `[pushgate] Note: ${event.note}`); + writeDetail(stdout, `Note: ${event.note}`); return; case "review-passed": - writeLine(stdout, "[pushgate] Local AI review passed with no findings."); + writeResultRow(stdout, "passed", "No findings"); return; case "finding": { - const label = event.finding.severity === "blocking" ? "BLOCK" : "WARN"; + const status = + event.finding.severity === "blocking" ? "blocked" : "warning"; const location = event.finding.line === "N/A" ? event.finding.file : `${event.finding.file}:${event.finding.line}`; - writeLine( + writeResultRow( stdout, - `[pushgate] ${label} AI ${event.finding.category} at ${location}.`, + status, + `AI ${humanizeCategory(event.finding.category)}`, + location, ); - writeLine(stdout, `[pushgate] Message: ${event.finding.message}`); - writeLine(stdout, `[pushgate] Suggestion: ${event.finding.suggestion}`); + writeDetail(stdout, `Message: ${event.finding.message}`); + writeDetail(stdout, `Suggestion: ${event.finding.suggestion}`); return; } case "review-summary": - writeLine( - stdout, - `[pushgate] Local AI review finished: ${String(event.summary.blockingCount)} blocking finding(s), ${String(event.summary.warningCount)} warning(s).`, - ); + if ( + event.summary.blockingCount > 0 || + event.summary.warningCount > 0 + ) { + writeDetail( + stdout, + `Finished with ${formatCount(event.summary.blockingCount, "blocking finding")} and ${formatCount(event.summary.warningCount, "warning")}.`, + ); + } return; case "advisory-continue": - writeLine(stdout, "[pushgate] Continuing because ai.mode is advisory."); + writeDetail(stdout, "Continuing because ai.mode is advisory."); return; case "provider-blocked": - writeLine( - stdout, - "[pushgate] Local AI is blocking in this repository. Fix the provider issue or use git -c pushgate.skip-ai-check=true push to bypass only the AI phase for one push.", - ); + writeLine(stdout); + writeLine(stdout, "Local AI is blocking in this repository."); + writeLine(stdout, "Fix the provider issue, or use `git -c pushgate.skip-ai-check=true push` to bypass only the AI phase for one push."); return; case "review-blocked": - writeLine( - stdout, - "[pushgate] Local AI review blocked the push. Fix the findings above or use git -c pushgate.skip-ai-check=true push to bypass only the AI phase for one push.", - ); + writeLine(stdout); + writeLine(stdout, "Local AI review blocked the push."); + writeLine(stdout, "Fix the findings above, or use `git -c pushgate.skip-ai-check=true push` to bypass only the AI phase for one push."); return; } } -function writeLine(stream: NodeJS.WritableStream, line: string): void { - stream.write(`${line}\n`); +function humanizeCategory(category: string): string { + return category.replace(/_/g, " "); } diff --git a/src/cli.ts b/src/cli.ts index 967f945..96ca616 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -3,7 +3,11 @@ import { fileURLToPath } from "node:url"; import { writePushgateError } from "./cli/errors.js"; import { parsePushCommandArgs } from "./cli/push-args.js"; -import { runGitPush } from "./git/push.js"; +import { + formatGitPushSuccessSummary, + resolveGitPushSuccessSummary, + runGitPush, +} from "./git/push.js"; import { buildGitPushArgs, SkipControlError, @@ -45,7 +49,7 @@ export async function main( io.stdout.write(`${HOOK_PROTOCOL}\n`); return 0; case "pre-push": - return runPrePushCommand(io); + return runPrePushCommand(args, io); case "push": return runPushCommand(args, io); default: @@ -57,9 +61,12 @@ export async function main( } } -async function runPrePushCommand(io: CliIO): Promise { +async function runPrePushCommand( + args: readonly string[], + io: CliIO, +): Promise { try { - return await runPrePushWorkflow(io); + return await runPrePushWorkflow({ ...io, hookArgs: args }); } catch (error) { writePushgateError(io.stderr, error); return 1; @@ -87,6 +94,10 @@ async function runPushCommand( }); if (result.code !== null) { + if (result.code === 0) { + await writeGitPushSuccessSummary(parsed.gitPushArgs, io); + } + return result.code; } @@ -99,6 +110,23 @@ async function runPushCommand( } } +async function writeGitPushSuccessSummary( + gitPushArgs: readonly string[], + io: CliIO, +): Promise { + try { + io.stdout.write( + formatGitPushSuccessSummary( + await resolveGitPushSuccessSummary(gitPushArgs, { + env: io.env, + }), + ), + ); + } catch { + // Post-push copy is best-effort; Git's completed push stays authoritative. + } +} + function writeUsageError( stderr: NodeJS.WritableStream, message: string, diff --git a/src/git/push.ts b/src/git/push.ts index e85890a..9339257 100644 --- a/src/git/push.ts +++ b/src/git/push.ts @@ -1,10 +1,22 @@ import { runInheritedCommand } from "../process/inherited-command.js"; +import { runCommand } from "../process/run-command.js"; export interface GitPushResult { code: number | null; signal: NodeJS.Signals | null; } +export interface GitPushSuccessSummary { + pullRequestUrl?: string; + upstream?: string; +} + +interface ParsedGitPushArgs { + branch?: string; + remote?: string; + setsUpstream: boolean; +} + export function runGitPush( args: readonly string[], options: { @@ -17,3 +29,265 @@ export function runGitPush( env: options.env, }); } + +export async function resolveGitPushSuccessSummary( + args: readonly string[], + options: { + env: NodeJS.ProcessEnv; + }, +): Promise { + const parsed = parseGitPushArgs(args); + const currentUpstream = parsed.setsUpstream + ? await readCurrentUpstream(options.env) + : undefined; + const branch = + parsed.branch ?? + branchFromUpstream(currentUpstream) ?? + (await readCurrentBranch(options.env)); + const remote = + parsed.remote ?? + remoteFromUpstream(currentUpstream) ?? + (branch ? await readConfiguredBranchRemote(branch, options.env) : undefined); + const upstream = parsed.setsUpstream + ? currentUpstream ?? upstreamFromParts(remote, branch) + : undefined; + const remoteUrl = remote ? await readRemoteUrl(remote, options.env) : undefined; + + return { + pullRequestUrl: + remoteUrl && branch + ? githubPullRequestUrl(remoteUrl, branch) + : undefined, + upstream, + }; +} + +export function formatGitPushSuccessSummary( + summary: GitPushSuccessSummary, +): string { + const rows = [" [ok] Branch pushed"]; + + if (summary.upstream) { + rows.push(` [ok] Upstream set: ${summary.upstream}`); + } + + let output = `\nPushing branch\n${rows.join("\n")}\n`; + + if (summary.pullRequestUrl) { + output += `\nCreate a pull request:\n ${summary.pullRequestUrl}\n`; + } + + return output; +} + +function parseGitPushArgs(args: readonly string[]): ParsedGitPushArgs { + const positionals: string[] = []; + let parseOptions = true; + let setsUpstream = false; + let skipNext = false; + + for (const arg of args) { + if (skipNext) { + skipNext = false; + continue; + } + + if (parseOptions && arg === "--") { + parseOptions = false; + continue; + } + + if (parseOptions && (arg === "-u" || arg === "--set-upstream")) { + setsUpstream = true; + continue; + } + + if (parseOptions && arg.startsWith("-")) { + skipNext = optionTakesSeparateValue(arg); + continue; + } + + positionals.push(arg); + } + + return { + branch: positionals[1] ? branchFromRefspec(positionals[1]) : undefined, + remote: positionals[0], + setsUpstream, + }; +} + +function optionTakesSeparateValue(arg: string): boolean { + return ( + arg === "--exec" || + arg === "--receive-pack" || + arg === "--repo" || + arg === "--push-option" || + arg === "-o" + ); +} + +function branchFromRefspec(refspec: string): string | undefined { + let branch = refspec.trim(); + + if (!branch || branch.includes("*")) { + return undefined; + } + + if (branch.startsWith("+")) { + branch = branch.slice(1); + } + + const remoteBranchSeparator = branch.lastIndexOf(":"); + if (remoteBranchSeparator >= 0) { + branch = branch.slice(remoteBranchSeparator + 1); + } + + if (!branch || branch === "HEAD") { + return undefined; + } + + if (branch.startsWith("refs/heads/")) { + return branch.slice("refs/heads/".length); + } + + if (branch.startsWith("refs/")) { + return undefined; + } + + return branch; +} + +async function readCurrentBranch( + env: NodeJS.ProcessEnv, +): Promise { + const branch = await readGitStdout(["rev-parse", "--abbrev-ref", "HEAD"], env); + + if (!branch || branch === "HEAD") { + return undefined; + } + + return branch; +} + +async function readCurrentUpstream( + env: NodeJS.ProcessEnv, +): Promise { + return readGitStdout( + ["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{upstream}"], + env, + ); +} + +async function readConfiguredBranchRemote( + branch: string, + env: NodeJS.ProcessEnv, +): Promise { + return readGitStdout(["config", "--get", `branch.${branch}.remote`], env); +} + +async function readRemoteUrl( + remote: string, + env: NodeJS.ProcessEnv, +): Promise { + return readGitStdout(["remote", "get-url", remote], env); +} + +async function readGitStdout( + args: readonly string[], + env: NodeJS.ProcessEnv, +): Promise { + try { + const result = await runCommand({ + args, + command: "git", + env, + }); + + if (result.code !== 0) { + return undefined; + } + + const output = result.stdout.trim(); + + return output ? output : undefined; + } catch { + return undefined; + } +} + +function upstreamFromParts( + remote: string | undefined, + branch: string | undefined, +): string | undefined { + return remote && branch ? `${remote}/${branch}` : undefined; +} + +function remoteFromUpstream(upstream: string | undefined): string | undefined { + const separator = upstream?.indexOf("/") ?? -1; + + return separator > 0 ? upstream?.slice(0, separator) : undefined; +} + +function branchFromUpstream(upstream: string | undefined): string | undefined { + const separator = upstream?.indexOf("/") ?? -1; + + return separator > 0 ? upstream?.slice(separator + 1) : undefined; +} + +function githubPullRequestUrl( + remoteUrl: string, + branch: string, +): string | undefined { + const repository = githubRepository(remoteUrl); + + if (!repository) { + return undefined; + } + + return `https://github.com/${repository.owner}/${repository.repo}/pull/new/${encodeURIComponent( + branch, + )}`; +} + +function githubRepository( + remoteUrl: string, +): { owner: string; repo: string } | undefined { + const trimmed = remoteUrl.trim(); + const sshMatch = /^git@github\.com:([^/\s]+)\/([^/\s]+?)(?:\.git)?$/i.exec( + trimmed, + ); + + if (sshMatch) { + return { + owner: sshMatch[1], + repo: sshMatch[2], + }; + } + + try { + const parsed = new URL(trimmed); + + if ( + parsed.protocol !== "https:" || + parsed.hostname.toLowerCase() !== "github.com" + ) { + return undefined; + } + + const pathParts = parsed.pathname.replace(/^\/|\/$/g, "").split("/"); + + if (pathParts.length !== 2) { + return undefined; + } + + const [owner, repoWithPossibleSuffix] = pathParts; + const repo = repoWithPossibleSuffix.endsWith(".git") + ? repoWithPossibleSuffix.slice(0, -".git".length) + : repoWithPossibleSuffix; + + return owner && repo ? { owner, repo } : undefined; + } catch { + return undefined; + } +} diff --git a/src/runner/transcript.ts b/src/runner/transcript.ts index acc8bfd..f3c306d 100644 --- a/src/runner/transcript.ts +++ b/src/runner/transcript.ts @@ -1,4 +1,14 @@ import type { ToolConfig } from "../config/index.js"; +import { + formatCount, + humanizeIdentifier, + writeDetail, + writeIndentedBlock, + writeLine, + writeResultRow, + writeSection, + type TerminalStatus, +} from "../terminal/format.js"; import type { ToolResult } from "./deterministic.js"; import type { BuiltInPolicyResult } from "./policies.js"; import type { DeterministicResultSummary } from "./summary.js"; @@ -16,90 +26,146 @@ export interface DeterministicTranscript { export function createDeterministicTranscript( stdout: NodeJS.WritableStream, ): DeterministicTranscript { + const warnings: string[] = []; + const blockers: string[] = []; + return { writeFailFast() { - writeLine( - stdout, - "[pushgate] Stopping deterministic checks after blocking failure because fail_fast is true.", - ); + writeDetail(stdout, "Stopped after a blocking failure because fail_fast is true."); }, writeNoChecks() { - writeLine(stdout, "[pushgate] No deterministic checks configured."); + writeSection(stdout, "Checks"); + writeResultRow(stdout, "skipped", "No checks configured"); + writeLine(stdout); }, writePolicyResult(result) { - const labelByStatus = { - blocked: "BLOCK", - passed: "PASS", - warning: "WARN", - } as const; - const detail = result.detail ? `: ${result.detail}` : ""; - - writeLine( - stdout, - `[pushgate] ${labelByStatus[result.status]} ${result.name}${detail}.`, - ); + writeCheckResult(result.name, result); }, writePluginResult(name, result) { - writeRunnableResult(name, result); + writeCheckResult(name, result); }, writeStart(checkCount) { - writeLine( - stdout, - `[pushgate] Running ${String(checkCount)} deterministic check(s).`, - ); + writeSection(stdout, "Checks"); + writeDetail(stdout, `Running ${formatCount(checkCount, "check")}.`); }, writeSummary(summary) { - writeLine( - stdout, - `[pushgate] Deterministic checks finished: ${String(summary.blockedCount)} blocking failure(s), ${String(summary.warningCount)} warning(s).`, - ); + writeLine(stdout); if (summary.blockedCount > 0) { writeLine( stdout, - "[pushgate] Fix the blocking command failures before pushing, or use git push --no-verify to bypass local hooks intentionally.", + `Checks completed with ${formatCount(summary.blockedCount, "blocking failure")} and ${formatCount(summary.warningCount, "warning")}.`, ); + writeLine(stdout); + writeSection(stdout, summary.blockedCount === 1 ? "Blocked" : "Blocked checks"); + + for (const blocker of blockers) { + writeDetail(stdout, `${blocker} failed and is configured as a blocking check.`); + } + + writeLine(stdout); + writeLine( + stdout, + "Fix the blocking failures above, or use `git push --no-verify` only when you intend to bypass local hooks.", + ); + return; } + + if (summary.warningCount > 0) { + writeLine( + stdout, + `Checks completed with ${formatCount(summary.warningCount, "non-blocking warning")}.`, + ); + writeLine(stdout); + writeSection(stdout, summary.warningCount === 1 ? "Warning" : "Warnings"); + + for (const warning of warnings) { + writeDetail(stdout, `${warning} failed, but this check does not block the push.`); + } + writeLine(stdout); + return; + } + + writeLine(stdout, "Checks passed."); + writeLine(stdout); }, writeToolResult(tool, result) { - writeRunnableResult(tool.name, result); + writeCheckResult(tool.name, result); }, }; - function writeRunnableResult(name: string, result: ToolResult): void { - if (result.status === "passed") { - writeLine(stdout, `[pushgate] PASS ${name}.`); - return; + function writeCheckResult( + name: string, + result: Pick, + ): void { + const display = displayCheck(name); + const detail = formatDetail(name, result.detail); + const status = mapStatus(result.status); + + writeResultRow(stdout, status, display.label, detail ?? display.detail); + + if (result.outputTail) { + writeDetail(stdout, "Command output:"); + writeIndentedBlock(stdout, result.outputTail.split("\n")); } - if (result.status === "skipped") { - writeLine(stdout, `[pushgate] SKIP ${name}: ${result.detail}.`); - return; + if (result.status === "warning") { + warnings.push(display.label); } - const label = result.status === "warning" ? "WARN" : "BLOCK"; + if (result.status === "blocked") { + blockers.push(display.label); + } + } +} - writeLine( - stdout, - `[pushgate] ${label} ${name}: ${result.detail ?? "command failed"}.`, - ); +function displayCheck(name: string): { detail?: string; label: string } { + if (name === "policy:diff_size") { + return { label: "Diff size" }; + } - if (result.outputTail) { - writeLine(stdout, "[pushgate] Command output:"); + if (name === "policy:forbidden_paths") { + return { label: "Forbidden paths" }; + } - for (const line of result.outputTail.split("\n")) { - writeLine(stdout, `[pushgate] ${line}`); - } + if (name === "plugin:gitleaks") { + return { detail: "gitleaks", label: "Secrets scan" }; + } + + return { label: humanizeIdentifier(name) }; +} + +function formatDetail(name: string, detail: string | undefined): string | undefined { + if (!detail) { + return undefined; + } + + if (name === "policy:diff_size") { + const passed = detail.match( + /^(\d+) changed line\(s\) within max_changed_lines (\d+)$/, + ); + + if (passed) { + return `${passed[1]} / ${passed[2]} changed lines`; } } + + return detail; } -function writeLine(stream: NodeJS.WritableStream, line: string): void { - stream.write(`${line}\n`); +function mapStatus(status: ToolResult["status"]): TerminalStatus { + const statusByResult = { + blocked: "blocked", + passed: "passed", + skipped: "skipped", + warning: "warning", + } as const satisfies Record; + + return statusByResult[status]; } diff --git a/src/terminal/format.ts b/src/terminal/format.ts new file mode 100644 index 0000000..75813f1 --- /dev/null +++ b/src/terminal/format.ts @@ -0,0 +1,211 @@ +export type TerminalStatus = "blocked" | "info" | "passed" | "skipped" | "warning"; + +interface TerminalStyleOptions { + env?: NodeJS.ProcessEnv; + stream?: NodeJS.WritableStream; +} + +const ANSI = { + blue: ["\u001B[34m", "\u001B[39m"], + bold: ["\u001B[1m", "\u001B[22m"], + dim: ["\u001B[2m", "\u001B[22m"], + green: ["\u001B[32m", "\u001B[39m"], + red: ["\u001B[31m", "\u001B[39m"], + yellow: ["\u001B[33m", "\u001B[39m"], +} as const; + +const LABEL_WIDTH = 18; + +export function writeHeader( + stream: NodeJS.WritableStream, + lines: readonly string[], +): void { + for (const line of lines) { + writeLine(stream, line); + } + + if (lines.length > 0) { + writeLine(stream, ""); + } +} + +export function writeSection( + stream: NodeJS.WritableStream, + title: string, + options: TerminalStyleOptions = {}, +): void { + writeLine(stream, style(title, "bold", withStream(stream, options))); +} + +export function writeResultRow( + stream: NodeJS.WritableStream, + status: TerminalStatus, + label: string, + detail?: string, + options: TerminalStyleOptions = {}, +): void { + const styleOptions = withStream(stream, options); + const symbol = statusSymbol(status, styleOptions); + const styledSymbol = styleStatus(symbol, status, styleOptions); + const paddedLabel = label.padEnd(LABEL_WIDTH, " "); + const suffix = detail ? ` ${detail}` : ""; + + writeLine(stream, ` ${styledSymbol} ${paddedLabel}${suffix}`.trimEnd()); +} + +export function writeDetail( + stream: NodeJS.WritableStream, + detail: string, +): void { + writeLine(stream, ` ${detail}`); +} + +export function writeIndentedBlock( + stream: NodeJS.WritableStream, + lines: readonly string[], +): void { + for (const line of lines) { + writeLine(stream, ` ${line}`); + } +} + +export function writeLine( + stream: NodeJS.WritableStream, + line = "", +): void { + stream.write(`${line}\n`); +} + +export function formatCount( + count: number, + singular: string, + plural = `${singular}s`, +): string { + return `${String(count)} ${count === 1 ? singular : plural}`; +} + +export function humanizeIdentifier(value: string): string { + const stripped = value.replace(/^(policy|plugin):/, ""); + const words = stripped + .replace(/[_-]+/g, " ") + .trim() + .split(/\s+/) + .filter(Boolean); + + if (words.length === 0) { + return value; + } + + return words + .map((word, index) => + index === 0 ? capitalize(word) : word.toLowerCase(), + ) + .join(" "); +} + +export function capitalize(value: string): string { + if (value.length === 0) { + return value; + } + + return `${value[0]?.toUpperCase() ?? ""}${value.slice(1)}`; +} + +function statusSymbol( + status: TerminalStatus, + options: TerminalStyleOptions, +): string { + const unicode = supportsUnicode(options); + + if (!unicode) { + return { + blocked: "[block]", + info: "[info]", + passed: "[ok]", + skipped: "[skip]", + warning: "[warn]", + }[status]; + } + + return { + blocked: "x", + info: "i", + passed: "\u2713", + skipped: "-", + warning: "!", + }[status]; +} + +function styleStatus( + value: string, + status: TerminalStatus, + options: TerminalStyleOptions, +): string { + switch (status) { + case "blocked": + return style(value, "red", options); + case "info": + return style(value, "blue", options); + case "passed": + return style(value, "green", options); + case "skipped": + return style(value, "dim", options); + case "warning": + return style(value, "yellow", options); + } +} + +function style( + value: string, + color: keyof typeof ANSI, + options: TerminalStyleOptions, +): string { + if (!supportsColor(options)) { + return value; + } + + const [open, close] = ANSI[color]; + return `${open}${value}${close}`; +} + +function supportsColor(options: TerminalStyleOptions): boolean { + const env = options.env ?? process.env; + + if (env.NO_COLOR !== undefined || env.NODE_DISABLE_COLORS !== undefined) { + return false; + } + + if (env.FORCE_COLOR !== undefined && env.FORCE_COLOR !== "0") { + return true; + } + + return (options.stream as { isTTY?: boolean } | undefined)?.isTTY === true; +} + +function supportsUnicode(options: TerminalStyleOptions): boolean { + const env = options.env ?? process.env; + + if (env.PUSHGATE_ASCII === "1") { + return false; + } + + if ((options.stream as { isTTY?: boolean } | undefined)?.isTTY !== true) { + return false; + } + + if (process.platform !== "win32") { + return env.TERM !== "linux"; + } + + return Boolean(env.WT_SESSION || env.TERMINUS_SUBLIME || env.CI); +} + +function withStream( + stream: NodeJS.WritableStream, + options: TerminalStyleOptions, +): TerminalStyleOptions { + return { + ...options, + stream: options.stream ?? stream, + }; +} diff --git a/src/version.ts b/src/version.ts new file mode 100644 index 0000000..230e2df --- /dev/null +++ b/src/version.ts @@ -0,0 +1 @@ +export const PUSHGATE_VERSION = "3.5.0"; // x-release-please-version diff --git a/src/workflows/pre-push.ts b/src/workflows/pre-push.ts index a512814..133f86d 100644 --- a/src/workflows/pre-push.ts +++ b/src/workflows/pre-push.ts @@ -1,3 +1,5 @@ +import { basename } from "node:path"; + import { runLocalAiReview } from "../ai/index.js"; import { loadConfig, type PushgateConfig } from "../config/index.js"; import { resolveGitRepositoryRoot } from "../git/repository.js"; @@ -7,6 +9,14 @@ import { } from "../path-policy/index.js"; import { runDeterministicChecks } from "../runner/deterministic.js"; import { resolveSkipControlState } from "../skip-controls.js"; +import { + writeDetail, + writeHeader, + writeLine, + writeResultRow, + writeSection, +} from "../terminal/format.js"; +import { PUSHGATE_VERSION } from "../version.js"; import { buildPrePushConfigDecision, buildPrePushRunDecision, @@ -24,6 +34,7 @@ import { export interface PrePushWorkflowIO { env: NodeJS.ProcessEnv; + hookArgs?: readonly string[]; stderr: NodeJS.WritableStream; stdin: NodeJS.ReadableStream; stdout: NodeJS.WritableStream; @@ -33,9 +44,14 @@ export interface PrePushWorkflowIO { export async function runPrePushWorkflow( io: PrePushWorkflowIO, ): Promise { - await drainStdin(io.stdin); + const hookContext = buildPrePushContext({ + args: io.hookArgs ?? [], + stdin: await readStdin(io.stdin), + }); const repoRoot = await resolveGitRepositoryRoot(io.env); + writePrePushHeader(io.stdout, repoRoot, hookContext); + const skipControls = await resolveSkipControlState(repoRoot, io.env); const configDecision = buildPrePushConfigDecision(skipControls); @@ -107,6 +123,8 @@ export async function runPrePushWorkflow( return 1; } + writeLine(io.stdout); + writeLine(io.stdout, "Pushgate passed. Git is pushing..."); return 0; } @@ -121,10 +139,18 @@ async function runLocalAiPhase( }, ): Promise<{ exitCode: number; warningCount: number }> { if (decision.kind === "skip") { - writeVisibleSkipReason(options.stdout, decision.reason); + const message = formatRunSkipReason(decision.reason); + + if (message !== null) { + writeSection(options.stdout, "AI review"); + writeResultRow(options.stdout, "skipped", message); + } + return { exitCode: 0, warningCount: 0 }; } + writeSection(options.stdout, "AI review"); + return await runLocalAiReview({ aiConfig: config.ai, changedFileResolution: requireChangedFileResolution( @@ -158,20 +184,20 @@ async function confirmWarningsBeforeContinuing(options: { if (confirmed) { options.stdout.write( - `[pushgate] Continuing with ${String(options.warningCount)} warning(s) from ${options.phase} after confirmation.\n`, + `Continuing with ${String(options.warningCount)} warning(s) from ${options.phase} after confirmation.\n`, ); return true; } options.stdout.write( - `[pushgate] Push blocked because ${options.phase} produced ${String(options.warningCount)} warning(s) and continuation was not confirmed.\n`, + `Push blocked because ${options.phase} produced ${String(options.warningCount)} warning(s) and continuation was not confirmed.\n`, ); return false; } catch (error) { if (error instanceof WarningConfirmationError) { - options.stdout.write(`[pushgate] ${error.message}\n`); + options.stdout.write(`${error.message}\n`); options.stdout.write( - "[pushgate] Push blocked because warning confirmation could not be collected.\n", + "Push blocked because warning confirmation could not be collected.\n", ); return false; } @@ -205,7 +231,7 @@ function writeVisibleSkipReason( const message = formatRunSkipReason(reason); if (message !== null) { - stdout.write(`[pushgate] ${message}\n`); + writeResultRow(stdout, "skipped", message); } } @@ -222,15 +248,77 @@ function requireChangedFileResolution( ); } -function drainStdin(stdin: NodeJS.ReadableStream): Promise { +interface PrePushContext { + branch?: string; + remote?: string; +} + +function writePrePushHeader( + stdout: NodeJS.WritableStream, + repoRoot: string, + context: PrePushContext, +): void { + const lines = [ + `Pushgate v${PUSHGATE_VERSION} - pre-push`, + `Repo: ${basename(repoRoot)}`, + ]; + + if (context.branch) { + lines.push(`Branch: ${context.branch}`); + } + + if (context.remote) { + lines.push(`Remote: ${context.remote}`); + } + + writeHeader(stdout, lines); +} + +function buildPrePushContext(options: { + args: readonly string[]; + stdin: string; +}): PrePushContext { + return { + branch: parseBranchFromPrePushInput(options.stdin), + remote: options.args[0], + }; +} + +function parseBranchFromPrePushInput(input: string): string | undefined { + for (const line of input.split(/\r?\n/)) { + const trimmed = line.trim(); + + if (!trimmed) { + continue; + } + + const [localRef] = trimmed.split(/\s+/, 1); + + if (localRef?.startsWith("refs/heads/")) { + return localRef.slice("refs/heads/".length); + } + } + + return undefined; +} + +function readStdin(stdin: NodeJS.ReadableStream): Promise { return new Promise((resolve, reject) => { if ((stdin as { isTTY?: boolean }).isTTY) { - resolve(); + resolve(""); return; } + let input = ""; + + stdin.setEncoding("utf8"); stdin.on("error", reject); - stdin.on("end", resolve); + stdin.on("data", (chunk: string) => { + input += chunk; + }); + stdin.on("end", () => { + resolve(input); + }); stdin.resume(); }); } diff --git a/src/workflows/terminal.ts b/src/workflows/terminal.ts index 86b4991..e20437f 100644 --- a/src/workflows/terminal.ts +++ b/src/workflows/terminal.ts @@ -53,7 +53,7 @@ function confirmWithInteractiveTerminal(question: string): boolean { writeSync( terminal.outputFd, - "[pushgate] Please answer yes(y) or no(n).\n", + "Please answer `y` or `n`.\n", ); } } catch (error) { @@ -68,7 +68,7 @@ function confirmWithInteractiveTerminal(question: string): boolean { } function formatYesNoPrompt(question: string): string { - return `${question} yes(y) / no(n) `; + return `${question} [y/N] `; } function normalizeAnswer(answer: string): "yes" | "no" | "invalid" { @@ -78,7 +78,7 @@ function normalizeAnswer(answer: string): "yes" | "no" | "invalid" { return "yes"; } - if (normalized === "n" || normalized === "no") { + if (normalized === "" || normalized === "n" || normalized === "no") { return "no"; } diff --git a/src/workflows/warning-confirmation.ts b/src/workflows/warning-confirmation.ts index a6f48f8..b1b17b6 100644 --- a/src/workflows/warning-confirmation.ts +++ b/src/workflows/warning-confirmation.ts @@ -35,7 +35,7 @@ export function createTerminalWarningConfirmer( return async (request) => { try { - return terminal.confirm(formatWarningQuestion(request)); + return terminal.confirm("Continue with push?"); } catch (error) { if (error instanceof InteractiveTerminalError) { throw new WarningConfirmationError( @@ -47,7 +47,3 @@ export function createTerminalWarningConfirmer( } }; } - -function formatWarningQuestion(request: WarningConfirmationRequest): string { - return `[pushgate] ${request.phase} produced ${String(request.warningCount)} warning(s). Continue with warnings?`; -} diff --git a/test/ai.test.ts b/test/ai.test.ts index 82f3b31..4938e72 100644 --- a/test/ai.test.ts +++ b/test/ai.test.ts @@ -817,7 +817,7 @@ test("builds and renders local AI verdict output without provider execution", () output.text(), /Note: Extracted the review JSON from a fenced code block/, ); - assert.match(output.text(), /BLOCK AI logic_errors at src\/changed\.ts:2/); + assert.match(output.text(), /\[block\] AI logic errors\s+src\/changed\.ts:2/); assert.match(output.text(), /Continuing because ai\.mode is advisory/); }); @@ -1065,8 +1065,8 @@ test("runs the Claude adapter through the provider interface with model selectio }); assert.equal(result.exitCode, 0, output.text()); - assert.match(output.text(), /Running local AI review with claude/); - assert.match(output.text(), /Local AI review passed with no findings/); + assert.match(output.text(), /Provider: Claude/); + assert.match(output.text(), /\[ok\] No findings/); assert.match(await readFile(promptPath, "utf8"), /=== DIFF ===/); assert.match(await readFile(promptPath, "utf8"), /"schema_version": 1/); const args = await readArgLines(argsPath); @@ -1872,7 +1872,7 @@ test("maps Copilot auth-like failures through advisory mode", async () => { }); assert.equal(result.exitCode, 0, output.text()); - assert.match(output.text(), /WARN local AI provider copilot failed/); + assert.match(output.text(), /\[warn\] Copilot provider/); assert.match(output.text(), /not authenticated or cannot access Copilot/); assert.match(output.text(), /Continuing because ai\.mode is advisory/); }); @@ -2096,7 +2096,7 @@ test("blocks local AI before provider invocation when changed-line guardrail is }); assert.equal(result.exitCode, 1, output.text()); - assert.match(output.text(), /BLOCK local AI because \d+ changed line\(s\) exceed ai\.max_changed_lines 1/); + assert.match(output.text(), /\[block\] Changed lines\s+\d+ changed lines exceed ai\.max_changed_lines 1/); assert.match(output.text(), /Local AI review blocked the push/); assert.doesNotMatch(output.text(), /provider claude failed/); }); @@ -2141,13 +2141,13 @@ test("reports unsupported local AI providers through the public gate", async () }); assert.equal(result.exitCode, 1, output.text()); - assert.match(output.text(), /BLOCK local AI provider openai failed/); + assert.match(output.text(), /\[block\] Openai provider/); assert.match( output.text(), /does not implement the configured AI provider "openai" yet/, ); assert.match(output.text(), /Local AI is blocking in this repository/); - assert.doesNotMatch(output.text(), /Running local AI review/); + assert.doesNotMatch(output.text(), /Provider: Openai/); }); test("skips local AI after prompt rendering when prompt token guardrail is exceeded", async () => { @@ -2180,7 +2180,7 @@ test("skips local AI after prompt rendering when prompt token guardrail is excee }); assert.equal(result.exitCode, 0, output.text()); - assert.match(output.text(), /Skipping local AI because the rendered prompt is approximately \d+ token\(s\), exceeding ai\.max_prompt_tokens 1/); + assert.match(output.text(), /\[skip\] Prompt budget\s+approximately \d+ tokens exceeds ai\.max_prompt_tokens 1/); assert.doesNotMatch(output.text(), /provider claude failed/); }); }); diff --git a/test/deterministic-runner.test.ts b/test/deterministic-runner.test.ts index ec51633..b86cbd6 100644 --- a/test/deterministic-runner.test.ts +++ b/test/deterministic-runner.test.ts @@ -232,7 +232,7 @@ test("blocks on blocking command failures", async () => { assert.equal(summary.exitCode, 1, output.text()); assert.equal(summary.results[0]?.status, "blocked"); - assert.match(output.text(), /BLOCK check: exited with code 2/); + assert.match(output.text(), /\[block\] Check\s+exited with code 2/); assert.match(output.text(), /lint failed/); assert.match(output.text(), /git push --no-verify/); }); @@ -253,7 +253,7 @@ test("warning-mode command failures do not block", async () => { assert.equal(summary.exitCode, 0, output.text()); assert.equal(summary.results[0]?.status, "warning"); - assert.match(output.text(), /WARN check: exited with code 7/); + assert.match(output.text(), /\[warn\] Check\s+exited with code 7/); }); }); @@ -376,7 +376,7 @@ test("runs Gitleaks plugin over the resolved branch commit range", async () => { assert.equal(summary.exitCode, 1, output.text()); assert.equal(summary.results[0]?.status, "blocked"); - assert.match(output.text(), /BLOCK plugin:gitleaks/); + assert.match(output.text(), /\[block\] Secrets scan/); assert.match(output.text(), /src\/config\.ts:3 \(generic-api-key\)/); const args = JSON.parse(await readFile(argsPath, "utf8")) as string[]; @@ -470,8 +470,8 @@ test("warning-mode Gitleaks findings do not stop later tools", async () => { ["warning", "passed"], ); assert.deepEqual(JSON.parse(await readFile(argsPath, "utf8")), []); - assert.match(output.text(), /WARN plugin:gitleaks/); - assert.match(output.text(), /PASS check/); + assert.match(output.text(), /\[warn\] Secrets scan/); + assert.match(output.text(), /\[ok\] Check/); }); }); @@ -498,10 +498,10 @@ test("runs built-in policies and makes warning versus blocking behavior explicit assert.equal(summary.exitCode, 1, output.text()); assert.equal(summary.results[0]?.status, "warning"); assert.equal(summary.results[1]?.status, "blocked"); - assert.match(output.text(), /WARN policy:diff_size/); - assert.match(output.text(), /BLOCK policy:forbidden_paths/); + assert.match(output.text(), /\[warn\] Diff size/); + assert.match(output.text(), /\[block\] Forbidden paths/); assert.match(output.text(), /src\/file with spaces\.ts \(src\/\*\*\)/); - assert.match(output.text(), /1 blocking failure\(s\), 1 warning\(s\)/); + assert.match(output.text(), /1 blocking failure and 1 warning/); }); }); @@ -523,8 +523,8 @@ test("warning-mode built-in policy failures do not block", async () => { assert.equal(summary.exitCode, 0, output.text()); assert.equal(summary.results[0]?.status, "warning"); - assert.match(output.text(), /WARN policy:diff_size/); - assert.match(output.text(), /0 blocking failure\(s\), 1 warning\(s\)/); + assert.match(output.text(), /\[warn\] Diff size/); + assert.match(output.text(), /1 non-blocking warning/); }); }); @@ -589,15 +589,21 @@ test("renders deterministic transcript without running commands", () => { assert.equal( output.text(), [ - "[pushgate] Running 3 deterministic check(s).", - "[pushgate] PASS policy:diff_size: 5 changed line(s) within max_changed_lines 10.", - "[pushgate] BLOCK check: exited with code 2.", - "[pushgate] Command output:", - "[pushgate] first line", - "[pushgate] second line", - "[pushgate] Stopping deterministic checks after blocking failure because fail_fast is true.", - "[pushgate] Deterministic checks finished: 1 blocking failure(s), 0 warning(s).", - "[pushgate] Fix the blocking command failures before pushing, or use git push --no-verify to bypass local hooks intentionally.", + "Checks", + " Running 3 checks.", + " [ok] Diff size 5 / 10 changed lines", + " [block] Check exited with code 2", + " Command output:", + " first line", + " second line", + " Stopped after a blocking failure because fail_fast is true.", + "", + "Checks completed with 1 blocking failure and 0 warnings.", + "", + "Blocked", + " Check failed and is configured as a blocking check.", + "", + "Fix the blocking failures above, or use `git push --no-verify` only when you intend to bypass local hooks.", "", ].join("\n"), ); diff --git a/test/hook.test.ts b/test/hook.test.ts index f362892..19e7e79 100644 --- a/test/hook.test.ts +++ b/test/hook.test.ts @@ -55,7 +55,7 @@ test("uses PUSHGATE_RUNNER when provided", async () => { "refs/heads/feature 0123456789 refs/heads/feature fedcba9876\n"; const result = await harness.runHook({ args: ["origin", "git@example.test:rootstrap/ai-pushgate.git"], - env: { PUSHGATE_RUNNER: runnerPath }, + env: { PUSHGATE_RUNNER: runnerPath, PUSHGATE_VERBOSE: "1" }, stdin, }); @@ -207,7 +207,9 @@ test("uses git config pushgate.runner during a real installed-hook push", async await harness.installInstalledHook(); await harness.addBareOrigin(); - const result = await harness.git(["push", "origin", "feature"]); + const result = await harness.git(["push", "origin", "feature"], { + env: { PUSHGATE_VERBOSE: "1" }, + }); const output = cleanHookOutput(result); assert.equal(result.code, 0, formatResult(result)); @@ -264,7 +266,7 @@ test("blocks a real installed-hook push on deterministic command failure", async const output = cleanHookOutput(result); assert.equal(result.code, 1, output); - assert.match(output, /BLOCK failing: exited with code 3/); + assert.match(output, /\[block\] Failing\s+exited with code 3/); assert.match(output, /tool failed/); }); }); @@ -332,7 +334,7 @@ test("skip-ai-check keeps deterministic checks running on a real installed-hook assert.equal(result.code, 0, output); assert.equal(await requiredArtifact(harness, "tool-ran.txt"), "ran\n"); - assert.match(output, /PASS record-tool/); + assert.match(output, /\[ok\] Record tool/); assert.match( output, /Skipping local AI because pushgate\.skip-ai-check=true/, @@ -385,8 +387,8 @@ test("invokes the Claude adapter on a real installed-hook push", async () => { const output = cleanHookOutput(result); assert.equal(result.code, 0, output); - assert.match(output, /Running local AI review with claude/); - assert.match(output, /Local AI review passed with no findings/); + assert.match(output, /Provider: Claude/); + assert.match(output, /\[ok\] No findings/); assert.match(await requiredArtifact(harness, "claude-prompt.txt"), /=== DIFF ===/); assert.match(await requiredArtifact(harness, "claude-prompt.txt"), /"schema_version": 1/); }); diff --git a/test/runner.test.ts b/test/runner.test.ts index c2174ab..b2ee32c 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -32,7 +32,7 @@ test("accepts pre-push args and drains Git hook stdin", async () => { ); assert.equal(result.code, 0, formatResult(result)); - assert.match(result.stdout, /No deterministic checks configured/); + assert.match(result.stdout, /\[skip\] No checks configured/); assert.equal(result.stderr, ""); }); }); @@ -62,12 +62,12 @@ test("runs built-in policies against resolved pre-push changed files", async () ); assert.equal(result.code, 1, formatResult(result)); - assert.match(result.stdout, /Running 2 deterministic check\(s\)/); - assert.match(result.stdout, /WARN policy:diff_size/); + assert.match(result.stdout, /Running 2 checks/); + assert.match(result.stdout, /\[warn\] Diff size/); assert.match(result.stdout, /3 changed line\(s\) exceed max_changed_lines 2/); - assert.match(result.stdout, /BLOCK policy:forbidden_paths/); + assert.match(result.stdout, /\[block\] Forbidden paths/); assert.match(result.stdout, /secrets\/token\.txt \(secrets\/\*\*\)/); - assert.match(result.stdout, /1 blocking failure\(s\), 1 warning\(s\)/); + assert.match(result.stdout, /1 blocking failure and 1 warning/); assert.equal(result.stderr, ""); }); }); @@ -81,8 +81,8 @@ test("Gitleaks plugin findings block the pre-push runner", async () => { ); assert.equal(result.code, 1, formatResult(result)); - assert.match(result.stdout, /Running 1 deterministic check\(s\)/); - assert.match(result.stdout, /BLOCK plugin:gitleaks/); + assert.match(result.stdout, /Running 1 check/); + assert.match(result.stdout, /\[block\] Secrets scan/); assert.match(result.stdout, /src\/secret\.txt:1 \(generic-api-key\)/); assert.doesNotMatch(result.stdout, /Running local AI review/); assert.equal(result.stderr, ""); @@ -101,7 +101,7 @@ test("deterministic warnings prompt before local AI runs", async () => { }); assert.equal(result.code, 0, formatResult(result)); - assert.match(result.stdout, /WARN warn-tool: exited with code 7/); + assert.match(result.stdout, /\[warn\] Warn tool\s+exited with code 7/); assert.match( result.stdout, /Continuing with 1 warning\(s\) from deterministic checks after confirmation/, @@ -125,7 +125,7 @@ test("declining deterministic warnings blocks the pre-push runner", async () => }); assert.equal(result.code, 1, formatResult(result)); - assert.match(result.stdout, /WARN warn-tool: exited with code 7/); + assert.match(result.stdout, /\[warn\] Warn tool\s+exited with code 7/); assert.match( result.stdout, /Push blocked because deterministic checks produced 1 warning\(s\) and continuation was not confirmed/, @@ -184,7 +184,7 @@ test("skip-ai-check keeps deterministic work and prints visible AI skip output", ); assert.equal(result.code, 0, formatResult(result)); - assert.match(result.stdout, /No deterministic checks configured/); + assert.match(result.stdout, /\[skip\] No checks configured/); assert.match( result.stdout, /Skipping local AI because pushgate\.skip-ai-check=true/, @@ -217,8 +217,8 @@ test("blocking local AI findings block the pre-push runner", async () => { ); assert.equal(result.code, 1, formatResult(result)); - assert.match(result.stdout, /Running local AI review with claude/); - assert.match(result.stdout, /BLOCK AI logic_errors at src\/changed\.ts:2-3/); + assert.match(result.stdout, /Provider: Claude/); + assert.match(result.stdout, /\[block\] AI logic errors\s+src\/changed\.ts:2-3/); assert.match(result.stdout, /Local AI review blocked the push/); assert.equal(result.stderr, ""); }); @@ -252,11 +252,11 @@ test("Copilot local AI warnings continue after confirmation", async () => { }); assert.equal(result.code, 0, formatResult(result)); - assert.match(result.stdout, /Running local AI review with copilot/); - assert.match(result.stdout, /WARN AI performance at src\/changed\.ts:2/); + assert.match(result.stdout, /Provider: Copilot/); + assert.match(result.stdout, /\[warn\] AI performance\s+src\/changed\.ts:2/); assert.match( result.stdout, - /Local AI review finished: 0 blocking finding\(s\), 1 warning\(s\)/, + /Finished with 0 blocking findings and 1 warning/, ); assert.match( result.stdout, @@ -297,7 +297,7 @@ test("declining local AI warnings blocks the pre-push runner", async () => { }); assert.equal(result.code, 1, formatResult(result)); - assert.match(result.stdout, /WARN AI performance at src\/changed\.ts:2/); + assert.match(result.stdout, /\[warn\] AI performance\s+src\/changed\.ts:2/); assert.match( result.stdout, /Push blocked because local AI review produced 1 warning\(s\) and continuation was not confirmed/, @@ -334,7 +334,7 @@ test("blocking local AI provider failures block the pre-push runner", async () = assert.equal(result.code, 1, formatResult(result)); assert.match( result.stdout, - /BLOCK local AI provider claude failed: Claude Code CLI was not found on PATH/, + /\[block\] Claude provider\s+Claude Code CLI was not found on PATH/, ); assert.match(result.stdout, /Local AI is blocking in this repository/); assert.equal(result.stderr, ""); @@ -365,7 +365,7 @@ test("default local AI mode is blocking in the pre-push runner", async () => { assert.equal(result.code, 1, formatResult(result)); assert.match( result.stdout, - /BLOCK local AI provider claude failed: Claude Code CLI was not found on PATH/, + /\[block\] Claude provider\s+Claude Code CLI was not found on PATH/, ); assert.equal(result.stderr, ""); }); @@ -398,7 +398,7 @@ test("advisory local AI provider failures continue after confirmation", async () assert.equal(result.code, 0, formatResult(result)); assert.match( result.stdout, - /WARN local AI provider claude failed: Claude Code CLI was not found on PATH/, + /\[warn\] Claude provider\s+Claude Code CLI was not found on PATH/, ); assert.match(result.stdout, /Continuing because ai.mode is advisory/); assert.match( @@ -438,7 +438,7 @@ test("AI changed-line guardrail blocks provider invocation visibly", async () => assert.equal(result.code, 1, formatResult(result)); assert.match( result.stdout, - /BLOCK local AI because \d+ changed line\(s\) exceed ai\.max_changed_lines 1/, + /\[block\] Changed lines\s+\d+ changed lines exceed ai\.max_changed_lines 1/, ); assert.match(result.stdout, /Local AI review blocked the push/); assert.doesNotMatch(result.stdout, /Running local AI review with claude/); @@ -522,6 +522,120 @@ test("push wrapper forwards Git args after -- without interpreting them as Pushg }); }); +test("push wrapper prints upstream and GitHub SSH PR URL after successful --set-upstream push", async () => { + await withGitPushSummaryStub( + { + currentBranch: "codex/cli-ux", + remoteUrl: "git@github.com:rootstrap/ai-pushgate.git", + upstream: "origin/codex/cli-ux", + }, + async ({ env, logPath, root }) => { + const result = await runRunner( + ["push", "--set-upstream", "origin", "codex/cli-ux"], + undefined, + { cwd: root, env }, + ); + + assert.equal(result.code, 0, formatResult(result)); + assert.match(result.stdout, /native git push output/); + assert.match(result.stdout, /Pushing branch/); + assert.match(result.stdout, / \[ok\] Branch pushed/); + assert.match( + result.stdout, + / \[ok\] Upstream set: origin\/codex\/cli-ux/, + ); + assert.match( + result.stdout, + /https:\/\/github\.com\/rootstrap\/ai-pushgate\/pull\/new\/codex%2Fcli-ux/, + ); + assert.match( + await readFile(logPath, "utf8"), + /call\tremote\tget-url\torigin/, + ); + assert.equal(result.stderr, ""); + }, + ); +}); + +test("push wrapper prints upstream and GitHub HTTPS PR URL after successful -u push", async () => { + await withGitPushSummaryStub( + { + currentBranch: "feature/http", + remoteUrl: "https://github.com/rootstrap/ai-pushgate", + upstream: "origin/feature/http", + }, + async ({ env, root }) => { + const result = await runRunner( + ["push", "-u", "origin", "feature/http"], + undefined, + { cwd: root, env }, + ); + + assert.equal(result.code, 0, formatResult(result)); + assert.match(result.stdout, /Pushing branch/); + assert.match(result.stdout, / \[ok\] Branch pushed/); + assert.match( + result.stdout, + / \[ok\] Upstream set: origin\/feature\/http/, + ); + assert.match( + result.stdout, + /https:\/\/github\.com\/rootstrap\/ai-pushgate\/pull\/new\/feature%2Fhttp/, + ); + assert.equal(result.stderr, ""); + }, + ); +}); + +test("push wrapper omits PR URL for non-GitHub remotes", async () => { + await withGitPushSummaryStub( + { + currentBranch: "feature", + remoteUrl: "https://gitlab.example.test/rootstrap/ai-pushgate.git", + upstream: "origin/feature", + }, + async ({ env, root }) => { + const result = await runRunner( + ["push", "-u", "origin", "feature"], + undefined, + { cwd: root, env }, + ); + + assert.equal(result.code, 0, formatResult(result)); + assert.match(result.stdout, /Pushing branch/); + assert.match(result.stdout, / \[ok\] Branch pushed/); + assert.match(result.stdout, / \[ok\] Upstream set: origin\/feature/); + assert.doesNotMatch(result.stdout, /Create a pull request:/); + assert.doesNotMatch(result.stdout, /pull\/new/); + assert.equal(result.stderr, ""); + }, + ); +}); + +test("push wrapper preserves failed Git push exit code without success rows", async () => { + await withGitPushSummaryStub( + { + currentBranch: "feature", + pushExit: "23", + remoteUrl: "git@github.com:rootstrap/ai-pushgate.git", + upstream: "origin/feature", + }, + async ({ env, root }) => { + const result = await runRunner( + ["push", "-u", "origin", "feature"], + undefined, + { cwd: root, env }, + ); + + assert.equal(result.code, 23, formatResult(result)); + assert.match(result.stdout, /native git push output/); + assert.doesNotMatch(result.stdout, /Pushing branch/); + assert.doesNotMatch(result.stdout, /Create a pull request:/); + assert.equal(result.stderr, ""); + }, + ); +}); + interface RunnerResult { code: number | null; stderr: string; @@ -1045,6 +1159,77 @@ async function withGitStub( } } +async function withGitPushSummaryStub( + options: { + currentBranch: string; + pushExit?: string; + remoteUrl: string; + upstream?: string; + }, + callback: (context: { + env: NodeJS.ProcessEnv; + logPath: string; + root: string; + }) => Promise, +): Promise { + const root = await mkdtemp(join(tmpdir(), "pushgate-push-summary-stub-")); + const binDir = join(root, "bin"); + const logPath = join(root, "git-calls.txt"); + + await mkdir(binDir, { recursive: true }); + await writeFile( + join(binDir, "git"), + [ + "#!/usr/bin/env bash", + "set -eu", + "{ printf 'call'; for arg in \"$@\"; do printf '\\t%s' \"$arg\"; done; printf '\\n'; } >> \"$PUSHGATE_GIT_CALLS_OUT\"", + "if [ \"${1:-}\" = 'push' ] || { [ \"${1:-}\" = '-c' ] && [ \"${3:-}\" = 'push' ]; }; then", + " printf 'native git push output\\n'", + " exit \"${PUSHGATE_GIT_PUSH_EXIT:-0}\"", + "fi", + "if [ \"${1:-}\" = 'rev-parse' ] && [ \"${2:-}\" = '--abbrev-ref' ] && [ \"${3:-}\" = '--symbolic-full-name' ]; then", + " if [ -n \"${PUSHGATE_GIT_UPSTREAM:-}\" ]; then", + " printf '%s\\n' \"$PUSHGATE_GIT_UPSTREAM\"", + " exit 0", + " fi", + " exit 1", + "fi", + "if [ \"${1:-}\" = 'rev-parse' ] && [ \"${2:-}\" = '--abbrev-ref' ] && [ \"${3:-}\" = 'HEAD' ]; then", + " printf '%s\\n' \"$PUSHGATE_GIT_CURRENT_BRANCH\"", + " exit 0", + "fi", + "if [ \"${1:-}\" = 'remote' ] && [ \"${2:-}\" = 'get-url' ]; then", + " printf '%s\\n' \"$PUSHGATE_GIT_REMOTE_URL\"", + " exit 0", + "fi", + "if [ \"${1:-}\" = 'config' ] && [ \"${2:-}\" = '--get' ]; then", + " printf 'origin\\n'", + " exit 0", + "fi", + "exit 19", + ].join("\n"), + ); + await chmod(join(binDir, "git"), 0o755); + + try { + await callback({ + env: { + ...process.env, + PATH: [binDir, process.env.PATH ?? ""].join(delimiter), + PUSHGATE_GIT_CALLS_OUT: logPath, + PUSHGATE_GIT_CURRENT_BRANCH: options.currentBranch, + PUSHGATE_GIT_PUSH_EXIT: options.pushExit ?? "0", + PUSHGATE_GIT_REMOTE_URL: options.remoteUrl, + PUSHGATE_GIT_UPSTREAM: options.upstream ?? "", + }, + logPath, + root, + }); + } finally { + await rm(root, { recursive: true, force: true }); + } +} + async function readArgLines(path: string): Promise { return (await readFile(path, "utf8")).trimEnd().split("\n"); } diff --git a/test/terminal.test.ts b/test/terminal.test.ts index e9f0955..ed7ebb6 100644 --- a/test/terminal.test.ts +++ b/test/terminal.test.ts @@ -5,50 +5,88 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import test from "node:test"; -import { createInteractiveTerminal } from "../src/workflows/terminal.js"; +import { + createInteractiveTerminal, + type InteractiveTerminal, +} from "../src/workflows/terminal.js"; + +test("interactive terminal accepts yes and no answers with empty input defaulting to no", async () => { + const results: boolean[] = []; + + const output = await withTerminalInput("y\nyes\nn\nno\n\n", (terminal) => { + results.push( + terminal.confirm("Continue with push?"), + terminal.confirm("Continue with push?"), + terminal.confirm("Continue with push?"), + terminal.confirm("Continue with push?"), + terminal.confirm("Continue with push?"), + ); + }); + + assert.deepEqual(results, [true, true, false, false, false]); + assert.equal(output, "Continue with push? [y/N] ".repeat(5)); +}); + +test("interactive terminal re-prompts after an invalid answer", async () => { + const output = await withTerminalInput("siu\ny\n", (terminal) => { + assert.equal(terminal.confirm("Continue with push?"), true); + }); + + assert.equal( + output, + [ + "Continue with push? [y/N] ", + "Please answer `y` or `n`.\n", + "Continue with push? [y/N] ", + ].join(""), + ); +}); test("interactive terminal consumes CRLF as one line ending", async () => { - await withTempDir(async (tempDir) => { + const output = await withTerminalInput("y\r\nn\r\n", (terminal) => { + assert.equal(terminal.confirm("Continue with push?"), true); + assert.equal(terminal.confirm("Continue with push?"), false); + }); + + assert.equal(output, "Continue with push? [y/N] ".repeat(2)); +}); + +async function withTempDir( + callback: (tempDir: string) => Promise, +): Promise { + const tempDir = await mkdtemp(join(tmpdir(), "pushgate-terminal-")); + + try { + return await callback(tempDir); + } finally { + await rm(tempDir, { force: true, recursive: true }); + } +} + +async function withTerminalInput( + input: string, + callback: (terminal: InteractiveTerminal) => void, +): Promise { + return await withTempDir(async (tempDir) => { const inputPath = join(tempDir, "input.txt"); const outputPath = join(tempDir, "output.txt"); - await writeFile(inputPath, "y\r\nn\r\n"); + await writeFile(inputPath, input); const inputFd = openSync(inputPath, "r"); const outputFd = openSync(outputPath, "w"); try { - await withAttachedTerminal(inputFd, outputFd, () => { - const terminal = createInteractiveTerminal(); - - assert.equal(terminal.confirm("[pushgate] First?"), true); - assert.equal(terminal.confirm("[pushgate] Second?"), false); + withAttachedTerminal(inputFd, outputFd, () => { + callback(createInteractiveTerminal()); }); } finally { closeSync(inputFd); closeSync(outputFd); } - assert.equal( - readFileSync(outputPath, "utf8"), - [ - "[pushgate] First? yes(y) / no(n) ", - "[pushgate] Second? yes(y) / no(n) ", - ].join(""), - ); + return readFileSync(outputPath, "utf8"); }); -}); - -async function withTempDir( - callback: (tempDir: string) => Promise, -): Promise { - const tempDir = await mkdtemp(join(tmpdir(), "pushgate-terminal-")); - - try { - await callback(tempDir); - } finally { - await rm(tempDir, { force: true, recursive: true }); - } } function withAttachedTerminal( diff --git a/test/warning-confirmation.test.ts b/test/warning-confirmation.test.ts index 5891137..ebbdf86 100644 --- a/test/warning-confirmation.test.ts +++ b/test/warning-confirmation.test.ts @@ -8,7 +8,7 @@ import { WarningConfirmationError, } from "../src/workflows/warning-confirmation.js"; -test("terminal warning confirmer asks one domain question through the terminal", async () => { +test("terminal warning confirmer asks the default-no push question through the terminal", async () => { const questions: string[] = []; const terminal: InteractiveTerminal = { confirm(question) { @@ -24,9 +24,7 @@ test("terminal warning confirmer asks one domain question through the terminal", }); assert.equal(confirmed, true); - assert.deepEqual(questions, [ - "[pushgate] deterministic checks produced 1 warning(s). Continue with warnings?", - ]); + assert.deepEqual(questions, ["Continue with push?"]); }); test("terminal warning confirmer maps terminal unavailability to a warning confirmation error", async () => { From f7cf9769c7cc09035859235717b9d40b8db75646 Mon Sep 17 00:00:00 2001 From: Pushgate Hook Harness Date: Sat, 27 Jun 2026 12:33:03 -0300 Subject: [PATCH 2/5] Enhance Pushgate CLI output formatting and success summary handling --- bin/pushgate.mjs | 299 ++++++++++++++++++++++---------------------- src/cli.ts | 18 +-- src/git/push.ts | 34 +++-- test/push.test.ts | 86 +++++++++++++ test/runner.test.ts | 6 +- 5 files changed, 271 insertions(+), 172 deletions(-) create mode 100644 test/push.test.ts diff --git a/bin/pushgate.mjs b/bin/pushgate.mjs index 82e37b7..30ff202 100755 --- a/bin/pushgate.mjs +++ b/bin/pushgate.mjs @@ -10061,6 +10061,136 @@ function runInheritedCommand(options) { }); } +// src/terminal/format.ts +var ANSI = { + blue: ["\x1B[34m", "\x1B[39m"], + bold: ["\x1B[1m", "\x1B[22m"], + dim: ["\x1B[2m", "\x1B[22m"], + green: ["\x1B[32m", "\x1B[39m"], + red: ["\x1B[31m", "\x1B[39m"], + yellow: ["\x1B[33m", "\x1B[39m"] +}; +var LABEL_WIDTH = 18; +function writeHeader(stream, lines) { + for (const line of lines) { + writeLine(stream, line); + } + if (lines.length > 0) { + writeLine(stream, ""); + } +} +function writeSection(stream, title, options = {}) { + writeLine(stream, style(title, "bold", withStream(stream, options))); +} +function writeResultRow(stream, status, label, detail, options = {}) { + const styleOptions = withStream(stream, options); + const symbol2 = statusSymbol(status, styleOptions); + const styledSymbol = styleStatus(symbol2, status, styleOptions); + const paddedLabel = label.padEnd(LABEL_WIDTH, " "); + const suffix = detail ? ` ${detail}` : ""; + writeLine(stream, ` ${styledSymbol} ${paddedLabel}${suffix}`.trimEnd()); +} +function writeDetail(stream, detail) { + writeLine(stream, ` ${detail}`); +} +function writeIndentedBlock(stream, lines) { + for (const line of lines) { + writeLine(stream, ` ${line}`); + } +} +function writeLine(stream, line = "") { + stream.write(`${line} +`); +} +function formatCount(count, singular, plural = `${singular}s`) { + return `${String(count)} ${count === 1 ? singular : plural}`; +} +function humanizeIdentifier(value) { + const stripped = value.replace(/^(policy|plugin):/, ""); + const words = stripped.replace(/[_-]+/g, " ").trim().split(/\s+/).filter(Boolean); + if (words.length === 0) { + return value; + } + return words.map( + (word, index) => index === 0 ? capitalize(word) : word.toLowerCase() + ).join(" "); +} +function capitalize(value) { + if (value.length === 0) { + return value; + } + return `${value[0]?.toUpperCase() ?? ""}${value.slice(1)}`; +} +function statusSymbol(status, options) { + const unicode = supportsUnicode(options); + if (!unicode) { + return { + blocked: "[block]", + info: "[info]", + passed: "[ok]", + skipped: "[skip]", + warning: "[warn]" + }[status]; + } + return { + blocked: "x", + info: "i", + passed: "\u2713", + skipped: "-", + warning: "!" + }[status]; +} +function styleStatus(value, status, options) { + switch (status) { + case "blocked": + return style(value, "red", options); + case "info": + return style(value, "blue", options); + case "passed": + return style(value, "green", options); + case "skipped": + return style(value, "dim", options); + case "warning": + return style(value, "yellow", options); + } +} +function style(value, color, options) { + if (!supportsColor(options)) { + return value; + } + const [open, close] = ANSI[color]; + return `${open}${value}${close}`; +} +function supportsColor(options) { + const env = options.env ?? process.env; + if (env.NO_COLOR !== void 0 || env.NODE_DISABLE_COLORS !== void 0) { + return false; + } + if (env.FORCE_COLOR !== void 0 && env.FORCE_COLOR !== "0") { + return true; + } + return options.stream?.isTTY === true; +} +function supportsUnicode(options) { + const env = options.env ?? process.env; + if (env.PUSHGATE_ASCII === "1") { + return false; + } + if (options.stream?.isTTY !== true) { + return false; + } + if (process.platform !== "win32") { + return env.TERM !== "linux"; + } + return Boolean(env.WT_SESSION || env.TERMINUS_SUBLIME || env.CI); +} +function withStream(stream, options) { + return { + ...options, + stream: options.stream ?? stream + }; +} + // src/git/push.ts function runGitPush(args, options) { return runInheritedCommand({ @@ -10081,22 +10211,18 @@ async function resolveGitPushSuccessSummary(args, options) { upstream }; } -function formatGitPushSuccessSummary(summary) { - const rows = [" [ok] Branch pushed"]; +function writeGitPushSuccessSummary(stream, summary, options = {}) { + writeLine(stream); + writeSection(stream, "Pushing branch", options); + writeResultRow(stream, "passed", "Branch pushed", void 0, options); if (summary.upstream) { - rows.push(` [ok] Upstream set: ${summary.upstream}`); + writeResultRow(stream, "passed", "Upstream set", summary.upstream, options); } - let output = ` -Pushing branch -${rows.join("\n")} -`; if (summary.pullRequestUrl) { - output += ` -Create a pull request: - ${summary.pullRequestUrl} -`; + writeLine(stream); + writeSection(stream, "Create a pull request:", options); + writeDetail(stream, summary.pullRequestUrl); } - return output; } function parseGitPushArgs(args) { const positionals = []; @@ -10141,6 +10267,9 @@ function branchFromRefspec(refspec) { } const remoteBranchSeparator = branch.lastIndexOf(":"); if (remoteBranchSeparator >= 0) { + if (remoteBranchSeparator === 0) { + return void 0; + } branch = branch.slice(remoteBranchSeparator + 1); } if (!branch || branch === "HEAD") { @@ -26683,136 +26812,6 @@ function countTextLines(text) { return text.endsWith("\n") ? newlineCount : newlineCount + 1; } -// src/terminal/format.ts -var ANSI = { - blue: ["\x1B[34m", "\x1B[39m"], - bold: ["\x1B[1m", "\x1B[22m"], - dim: ["\x1B[2m", "\x1B[22m"], - green: ["\x1B[32m", "\x1B[39m"], - red: ["\x1B[31m", "\x1B[39m"], - yellow: ["\x1B[33m", "\x1B[39m"] -}; -var LABEL_WIDTH = 18; -function writeHeader(stream, lines) { - for (const line of lines) { - writeLine(stream, line); - } - if (lines.length > 0) { - writeLine(stream, ""); - } -} -function writeSection(stream, title, options = {}) { - writeLine(stream, style(title, "bold", withStream(stream, options))); -} -function writeResultRow(stream, status, label, detail, options = {}) { - const styleOptions = withStream(stream, options); - const symbol2 = statusSymbol(status, styleOptions); - const styledSymbol = styleStatus(symbol2, status, styleOptions); - const paddedLabel = label.padEnd(LABEL_WIDTH, " "); - const suffix = detail ? ` ${detail}` : ""; - writeLine(stream, ` ${styledSymbol} ${paddedLabel}${suffix}`.trimEnd()); -} -function writeDetail(stream, detail) { - writeLine(stream, ` ${detail}`); -} -function writeIndentedBlock(stream, lines) { - for (const line of lines) { - writeLine(stream, ` ${line}`); - } -} -function writeLine(stream, line = "") { - stream.write(`${line} -`); -} -function formatCount(count, singular, plural = `${singular}s`) { - return `${String(count)} ${count === 1 ? singular : plural}`; -} -function humanizeIdentifier(value) { - const stripped = value.replace(/^(policy|plugin):/, ""); - const words = stripped.replace(/[_-]+/g, " ").trim().split(/\s+/).filter(Boolean); - if (words.length === 0) { - return value; - } - return words.map( - (word, index) => index === 0 ? capitalize(word) : word.toLowerCase() - ).join(" "); -} -function capitalize(value) { - if (value.length === 0) { - return value; - } - return `${value[0]?.toUpperCase() ?? ""}${value.slice(1)}`; -} -function statusSymbol(status, options) { - const unicode = supportsUnicode(options); - if (!unicode) { - return { - blocked: "[block]", - info: "[info]", - passed: "[ok]", - skipped: "[skip]", - warning: "[warn]" - }[status]; - } - return { - blocked: "x", - info: "i", - passed: "\u2713", - skipped: "-", - warning: "!" - }[status]; -} -function styleStatus(value, status, options) { - switch (status) { - case "blocked": - return style(value, "red", options); - case "info": - return style(value, "blue", options); - case "passed": - return style(value, "green", options); - case "skipped": - return style(value, "dim", options); - case "warning": - return style(value, "yellow", options); - } -} -function style(value, color, options) { - if (!supportsColor(options)) { - return value; - } - const [open, close] = ANSI[color]; - return `${open}${value}${close}`; -} -function supportsColor(options) { - const env = options.env ?? process.env; - if (env.NO_COLOR !== void 0 || env.NODE_DISABLE_COLORS !== void 0) { - return false; - } - if (env.FORCE_COLOR !== void 0 && env.FORCE_COLOR !== "0") { - return true; - } - return options.stream?.isTTY === true; -} -function supportsUnicode(options) { - const env = options.env ?? process.env; - if (env.PUSHGATE_ASCII === "1") { - return false; - } - if (options.stream?.isTTY !== true) { - return false; - } - if (process.platform !== "win32") { - return env.TERM !== "linux"; - } - return Boolean(env.WT_SESSION || env.TERMINUS_SUBLIME || env.CI); -} -function withStream(stream, options) { - return { - ...options, - stream: options.stream ?? stream - }; -} - // src/ai/transcript.ts function renderLocalAiTranscript(events, stdout) { for (const event of events) { @@ -28141,7 +28140,7 @@ async function runPushCommand(args, io) { }); if (result.code !== null) { if (result.code === 0) { - await writeGitPushSuccessSummary(parsed.gitPushArgs, io); + await writeResolvedGitPushSuccessSummary(parsed.gitPushArgs, io); } return result.code; } @@ -28153,14 +28152,14 @@ async function runPushCommand(args, io) { return 1; } } -async function writeGitPushSuccessSummary(gitPushArgs, io) { +async function writeResolvedGitPushSuccessSummary(gitPushArgs, io) { try { - io.stdout.write( - formatGitPushSuccessSummary( - await resolveGitPushSuccessSummary(gitPushArgs, { - env: io.env - }) - ) + writeGitPushSuccessSummary( + io.stdout, + await resolveGitPushSuccessSummary(gitPushArgs, { + env: io.env + }), + { env: io.env } ); } catch { } diff --git a/src/cli.ts b/src/cli.ts index 96ca616..0b7da1c 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -4,9 +4,9 @@ import { fileURLToPath } from "node:url"; import { writePushgateError } from "./cli/errors.js"; import { parsePushCommandArgs } from "./cli/push-args.js"; import { - formatGitPushSuccessSummary, resolveGitPushSuccessSummary, runGitPush, + writeGitPushSuccessSummary, } from "./git/push.js"; import { buildGitPushArgs, @@ -95,7 +95,7 @@ async function runPushCommand( if (result.code !== null) { if (result.code === 0) { - await writeGitPushSuccessSummary(parsed.gitPushArgs, io); + await writeResolvedGitPushSuccessSummary(parsed.gitPushArgs, io); } return result.code; @@ -110,17 +110,17 @@ async function runPushCommand( } } -async function writeGitPushSuccessSummary( +async function writeResolvedGitPushSuccessSummary( gitPushArgs: readonly string[], io: CliIO, ): Promise { try { - io.stdout.write( - formatGitPushSuccessSummary( - await resolveGitPushSuccessSummary(gitPushArgs, { - env: io.env, - }), - ), + writeGitPushSuccessSummary( + io.stdout, + await resolveGitPushSuccessSummary(gitPushArgs, { + env: io.env, + }), + { env: io.env }, ); } catch { // Post-push copy is best-effort; Git's completed push stays authoritative. diff --git a/src/git/push.ts b/src/git/push.ts index 9339257..85c217d 100644 --- a/src/git/push.ts +++ b/src/git/push.ts @@ -1,5 +1,11 @@ import { runInheritedCommand } from "../process/inherited-command.js"; import { runCommand } from "../process/run-command.js"; +import { + writeDetail, + writeLine, + writeResultRow, + writeSection, +} from "../terminal/format.js"; export interface GitPushResult { code: number | null; @@ -62,22 +68,26 @@ export async function resolveGitPushSuccessSummary( }; } -export function formatGitPushSuccessSummary( +export function writeGitPushSuccessSummary( + stream: NodeJS.WritableStream, summary: GitPushSuccessSummary, -): string { - const rows = [" [ok] Branch pushed"]; + options: { + env?: NodeJS.ProcessEnv; + } = {}, +): void { + writeLine(stream); + writeSection(stream, "Pushing branch", options); + writeResultRow(stream, "passed", "Branch pushed", undefined, options); if (summary.upstream) { - rows.push(` [ok] Upstream set: ${summary.upstream}`); + writeResultRow(stream, "passed", "Upstream set", summary.upstream, options); } - let output = `\nPushing branch\n${rows.join("\n")}\n`; - if (summary.pullRequestUrl) { - output += `\nCreate a pull request:\n ${summary.pullRequestUrl}\n`; + writeLine(stream); + writeSection(stream, "Create a pull request:", options); + writeDetail(stream, summary.pullRequestUrl); } - - return output; } function parseGitPushArgs(args: readonly string[]): ParsedGitPushArgs { @@ -127,7 +137,7 @@ function optionTakesSeparateValue(arg: string): boolean { ); } -function branchFromRefspec(refspec: string): string | undefined { +export function branchFromRefspec(refspec: string): string | undefined { let branch = refspec.trim(); if (!branch || branch.includes("*")) { @@ -140,6 +150,10 @@ function branchFromRefspec(refspec: string): string | undefined { const remoteBranchSeparator = branch.lastIndexOf(":"); if (remoteBranchSeparator >= 0) { + if (remoteBranchSeparator === 0) { + return undefined; + } + branch = branch.slice(remoteBranchSeparator + 1); } diff --git a/test/push.test.ts b/test/push.test.ts new file mode 100644 index 0000000..8446371 --- /dev/null +++ b/test/push.test.ts @@ -0,0 +1,86 @@ +import assert from "node:assert/strict"; +import { Writable } from "node:stream"; +import test from "node:test"; + +import { + branchFromRefspec, + writeGitPushSuccessSummary, +} from "../src/git/push.js"; + +test("writeGitPushSuccessSummary uses shared terminal result formatting", () => { + const output = captureOutput(); + + Object.defineProperty(output.stream, "isTTY", { + configurable: true, + value: true, + }); + + writeGitPushSuccessSummary( + output.stream, + { + pullRequestUrl: "https://github.com/rootstrap/ai-pushgate/pull/new/main", + upstream: "origin/main", + }, + { + env: { + CI: "1", + FORCE_COLOR: "1", + TERM: "xterm-256color", + }, + }, + ); + + assert.match(output.text(), /\u001B\[1mPushing branch\u001B\[22m/); + assert.match(output.text(), / \u001B\[32m✓\u001B\[39m Branch pushed/); + assert.match( + output.text(), + / \u001B\[32m✓\u001B\[39m Upstream set\s+origin\/main/, + ); + assert.match( + output.text(), + /\u001B\[1mCreate a pull request:\u001B\[22m/, + ); +}); + +test("branchFromRefspec returns the pushed destination branch", () => { + assert.equal(branchFromRefspec("feature"), "feature"); + assert.equal(branchFromRefspec("+feature"), "feature"); + assert.equal(branchFromRefspec("refs/heads/feature"), "feature"); + assert.equal(branchFromRefspec("feature:release"), "release"); + assert.equal(branchFromRefspec("HEAD:refs/heads/target"), "target"); + assert.equal( + branchFromRefspec("+refs/heads/src:refs/heads/dst"), + "dst", + ); +}); + +test("branchFromRefspec ignores refspecs that cannot name a pushed branch", () => { + assert.equal(branchFromRefspec(""), undefined); + assert.equal(branchFromRefspec(" "), undefined); + assert.equal(branchFromRefspec("HEAD"), undefined); + assert.equal(branchFromRefspec("refs/heads/*:refs/heads/*"), undefined); + assert.equal(branchFromRefspec(":refs/heads/old"), undefined); + assert.equal(branchFromRefspec("+:refs/heads/old"), undefined); + assert.equal(branchFromRefspec("refs/heads/source:"), undefined); + assert.equal(branchFromRefspec("refs/tags/v1.0.0"), undefined); +}); + +function captureOutput(): { + stream: Writable; + text(): string; +} { + let output = ""; + const stream = new Writable({ + write(chunk, _encoding, callback) { + output += chunk.toString(); + callback(); + }, + }); + + return { + stream, + text() { + return output; + }, + }; +} diff --git a/test/runner.test.ts b/test/runner.test.ts index b2ee32c..d2cbee8 100644 --- a/test/runner.test.ts +++ b/test/runner.test.ts @@ -542,7 +542,7 @@ test("push wrapper prints upstream and GitHub SSH PR URL after successful --set- assert.match(result.stdout, / \[ok\] Branch pushed/); assert.match( result.stdout, - / \[ok\] Upstream set: origin\/codex\/cli-ux/, + / \[ok\] Upstream set\s+origin\/codex\/cli-ux/, ); assert.match( result.stdout, @@ -576,7 +576,7 @@ test("push wrapper prints upstream and GitHub HTTPS PR URL after successful -u p assert.match(result.stdout, / \[ok\] Branch pushed/); assert.match( result.stdout, - / \[ok\] Upstream set: origin\/feature\/http/, + / \[ok\] Upstream set\s+origin\/feature\/http/, ); assert.match( result.stdout, @@ -604,7 +604,7 @@ test("push wrapper omits PR URL for non-GitHub remotes", async () => { assert.equal(result.code, 0, formatResult(result)); assert.match(result.stdout, /Pushing branch/); assert.match(result.stdout, / \[ok\] Branch pushed/); - assert.match(result.stdout, / \[ok\] Upstream set: origin\/feature/); + assert.match(result.stdout, / \[ok\] Upstream set\s+origin\/feature/); assert.doesNotMatch(result.stdout, /Create a pull request:/); assert.doesNotMatch(result.stdout, /pull\/new/); assert.equal(result.stderr, ""); From 214bf92bef2557f1628d12c2ee1dcecdd55cc5bb Mon Sep 17 00:00:00 2001 From: Pushgate Hook Harness Date: Sat, 27 Jun 2026 15:51:22 -0300 Subject: [PATCH 3/5] Enhance process error handling in runCapturedCommand --- bin/pushgate.mjs | 45 ++++++++++++++++++++++++++++-- src/ai/review-context.ts | 9 +++++- src/process/captured-command.ts | 49 +++++++++++++++++++++++++++++++-- test/process.test.ts | 30 ++++++++++++++++++++ 4 files changed, 127 insertions(+), 6 deletions(-) diff --git a/bin/pushgate.mjs b/bin/pushgate.mjs index 30ff202..756b12a 100755 --- a/bin/pushgate.mjs +++ b/bin/pushgate.mjs @@ -9534,12 +9534,15 @@ function runCapturedCommand(options) { const stdoutBuffers = []; let stdout = ""; let stderr = ""; + let exited = false; let timedOut = false; let settled = false; let killTimer; let timeoutTimer; + const useProcessGroup = shouldUseProcessGroup(options); const child = spawn(options.command, [...options.args ?? []], { cwd: options.cwd, + detached: useProcessGroup, env: options.env, shell: options.shell, stdio: [options.stdin === void 0 ? "ignore" : "pipe", "pipe", "pipe"] @@ -9562,9 +9565,11 @@ function runCapturedCommand(options) { if (options.timeoutMs !== void 0) { timeoutTimer = setTimeout(() => { timedOut = true; - child.kill("SIGTERM"); + signalChild("SIGTERM"); killTimer = setTimeout(() => { - child.kill("SIGKILL"); + if (useProcessGroup || !exited) { + signalChild("SIGKILL"); + } }, options.killGraceMs ?? 0); }, options.timeoutMs); } @@ -9601,6 +9606,13 @@ function runCapturedCommand(options) { stdout: capturedStdout() }); }); + child.on("exit", () => { + exited = true; + if (!useProcessGroup && killTimer) { + clearTimeout(killTimer); + killTimer = void 0; + } + }); child.on("close", (code, signal) => { if (timedOut) { finish({ @@ -9637,8 +9649,30 @@ function runCapturedCommand(options) { } child.stdin.end(options.stdin); } + function signalChild(signal) { + if (child.pid === void 0) { + return; + } + try { + if (useProcessGroup) { + process.kill(-child.pid, signal); + return; + } + child.kill(signal); + } catch (error51) { + if (!isMissingProcessError(error51)) { + throw error51; + } + } + } }); } +function shouldUseProcessGroup(options) { + return options.timeoutMs !== void 0 && process.platform !== "win32"; +} +function isMissingProcessError(error51) { + return error51 !== null && typeof error51 === "object" && "code" in error51 && error51.code === "ESRCH"; +} function appendCaptured(current, next, outputCaptureLimit) { return outputCaptureLimit === void 0 ? current + next : appendCapped(current, next, outputCaptureLimit); } @@ -26746,7 +26780,12 @@ async function collectReviewDiff(options) { if (error51 instanceof GitCommandError) { const stderr = error51.result.stderr.trim(); throw new Error( - `git diff failed while building the local AI review payload.${stderr ? ` ${stderr}` : ""}` + [ + "git diff failed while building the local AI review payload.", + `code=${String(error51.result.code)}`, + `signal=${String(error51.result.signal)}`, + stderr ? `stderr=${stderr}` : "" + ].filter(Boolean).join(" ") ); } throw error51; diff --git a/src/ai/review-context.ts b/src/ai/review-context.ts index ca4c02b..5287e50 100644 --- a/src/ai/review-context.ts +++ b/src/ai/review-context.ts @@ -92,7 +92,14 @@ async function collectReviewDiff(options: { const stderr = error.result.stderr.trim(); throw new Error( - `git diff failed while building the local AI review payload.${stderr ? ` ${stderr}` : ""}`, + [ + "git diff failed while building the local AI review payload.", + `code=${String(error.result.code)}`, + `signal=${String(error.result.signal)}`, + stderr ? `stderr=${stderr}` : "", + ] + .filter(Boolean) + .join(" "), ); } diff --git a/src/process/captured-command.ts b/src/process/captured-command.ts index edeb286..3de7665 100644 --- a/src/process/captured-command.ts +++ b/src/process/captured-command.ts @@ -63,13 +63,16 @@ export function runCapturedCommand( const stdoutBuffers: Buffer[] = []; let stdout = ""; let stderr = ""; + let exited = false; let timedOut = false; let settled = false; let killTimer: NodeJS.Timeout | undefined; let timeoutTimer: NodeJS.Timeout | undefined; + const useProcessGroup = shouldUseProcessGroup(options); const child = spawn(options.command, [...(options.args ?? [])], { cwd: options.cwd, + detached: useProcessGroup, env: options.env, shell: options.shell, stdio: [options.stdin === undefined ? "ignore" : "pipe", "pipe", "pipe"], @@ -101,9 +104,11 @@ export function runCapturedCommand( if (options.timeoutMs !== undefined) { timeoutTimer = setTimeout(() => { timedOut = true; - child.kill("SIGTERM"); + signalChild("SIGTERM"); killTimer = setTimeout(() => { - child.kill("SIGKILL"); + if (useProcessGroup || !exited) { + signalChild("SIGKILL"); + } }, options.killGraceMs ?? 0); }, options.timeoutMs); } @@ -143,6 +148,14 @@ export function runCapturedCommand( stdout: capturedStdout(), }); }); + child.on("exit", () => { + exited = true; + + if (!useProcessGroup && killTimer) { + clearTimeout(killTimer); + killTimer = undefined; + } + }); child.on("close", (code, signal) => { if (timedOut) { finish({ @@ -184,9 +197,41 @@ export function runCapturedCommand( child.stdin.end(options.stdin); } + + function signalChild(signal: NodeJS.Signals): void { + if (child.pid === undefined) { + return; + } + + try { + if (useProcessGroup) { + process.kill(-child.pid, signal); + return; + } + + child.kill(signal); + } catch (error) { + if (!isMissingProcessError(error)) { + throw error; + } + } + } }); } +function shouldUseProcessGroup(options: CapturedCommandOptions): boolean { + return options.timeoutMs !== undefined && process.platform !== "win32"; +} + +function isMissingProcessError(error: unknown): boolean { + return ( + error !== null && + typeof error === "object" && + "code" in error && + (error as { code?: unknown }).code === "ESRCH" + ); +} + function appendCaptured( current: string, next: string, diff --git a/test/process.test.ts b/test/process.test.ts index 8f36024..ded1eb0 100644 --- a/test/process.test.ts +++ b/test/process.test.ts @@ -101,6 +101,36 @@ test("runTimedCommand reports timeout with captured output tail", async () => { } }); +test("runTimedCommand terminates timed-out process groups", async () => { + if (process.platform === "win32") { + return; + } + + const started = Date.now(); + const result = await runTimedCommand({ + args: [ + "-e", + [ + "const { spawn } = require('node:child_process');", + "spawn(process.execPath, ['-e', 'setTimeout(() => {}, 5000);'], { stdio: ['ignore', 'inherit', 'inherit'] });", + "setInterval(() => {}, 1000);", + ].join(" "), + ], + command: process.execPath, + cwd: process.cwd(), + env: process.env, + killGraceMs: 50, + timeoutSeconds: 1, + }); + const elapsedMs = Date.now() - started; + + assert.equal(result.kind, "timeout"); + assert.ok( + elapsedMs < 3_000, + `expected process group cleanup before grandchild sleep finished, took ${elapsedMs}ms`, + ); +}); + test("runTimedCommand stdin broken pipes do not override close results", async () => { const result = await runTimedCommand({ args: ["-e", "process.exit(0);"], From 1c6017321cf1b9d17bf5d6cd0dbca53063929305 Mon Sep 17 00:00:00 2001 From: Pushgate Hook Harness Date: Sat, 27 Jun 2026 17:20:03 -0300 Subject: [PATCH 4/5] Refactor captured command handling and enhance Pushgate CLI output formatting --- bin/pushgate.mjs | 115 ++++++++++++++-------- hook/pre-push | 29 +++++- src/git/push.ts | 43 +++++++- src/process/captured-command.ts | 33 ++++--- src/terminal/format.ts | 59 ++++++----- test/deterministic-runner.test.ts | 6 ++ test/install.test.ts | 2 +- test/process.test.ts | 157 +++++++++++++++++++++++++++--- test/push.test.ts | 107 ++++++++++++++++++++ 9 files changed, 449 insertions(+), 102 deletions(-) diff --git a/bin/pushgate.mjs b/bin/pushgate.mjs index 756b12a..2e94d31 100755 --- a/bin/pushgate.mjs +++ b/bin/pushgate.mjs @@ -9534,7 +9534,6 @@ function runCapturedCommand(options) { const stdoutBuffers = []; let stdout = ""; let stderr = ""; - let exited = false; let timedOut = false; let settled = false; let killTimer; @@ -9549,6 +9548,14 @@ function runCapturedCommand(options) { }); const capturedStdout = () => outputEncoding === "buffer" ? Buffer.concat(stdoutBuffers) : stdout; const capturedOutputTail = () => outputEncoding === "utf8" && options.outputTailLimit !== void 0 ? formatOutputTail(stdout, stderr, options.outputTailLimit) : void 0; + const finishTimeout = () => { + finish({ + kind: "timeout", + outputTail: capturedOutputTail(), + stderr, + stdout: capturedStdout() + }); + }; const finish = (result) => { if (settled) { return; @@ -9567,9 +9574,8 @@ function runCapturedCommand(options) { timedOut = true; signalChild("SIGTERM"); killTimer = setTimeout(() => { - if (useProcessGroup || !exited) { - signalChild("SIGKILL"); - } + signalChild("SIGKILL"); + killTimer = void 0; }, options.killGraceMs ?? 0); }, options.timeoutMs); } @@ -9607,7 +9613,6 @@ function runCapturedCommand(options) { }); }); child.on("exit", () => { - exited = true; if (!useProcessGroup && killTimer) { clearTimeout(killTimer); killTimer = void 0; @@ -9615,14 +9620,17 @@ function runCapturedCommand(options) { }); child.on("close", (code, signal) => { if (timedOut) { - finish({ - kind: "timeout", - outputTail: capturedOutputTail(), - stderr, - stdout: capturedStdout() - }); + if (useProcessGroup && killTimer) { + clearTimeout(killTimer); + killTimer = void 0; + signalChild("SIGKILL"); + } + finishTimeout(); return; } + if (useProcessGroup) { + signalChild("SIGKILL"); + } finish({ code, kind: "completed", @@ -10104,6 +10112,27 @@ var ANSI = { red: ["\x1B[31m", "\x1B[39m"], yellow: ["\x1B[33m", "\x1B[39m"] }; +var ASCII_STATUS_SYMBOLS = { + blocked: "[block]", + info: "[info]", + passed: "[ok]", + skipped: "[skip]", + warning: "[warn]" +}; +var UNICODE_STATUS_SYMBOLS = { + blocked: "x", + info: "i", + passed: "\u2713", + skipped: "-", + warning: "!" +}; +var STATUS_COLORS = { + blocked: "red", + info: "blue", + passed: "green", + skipped: "dim", + warning: "yellow" +}; var LABEL_WIDTH = 18; function writeHeader(stream, lines) { for (const line of lines) { @@ -10156,37 +10185,10 @@ function capitalize(value) { return `${value[0]?.toUpperCase() ?? ""}${value.slice(1)}`; } function statusSymbol(status, options) { - const unicode = supportsUnicode(options); - if (!unicode) { - return { - blocked: "[block]", - info: "[info]", - passed: "[ok]", - skipped: "[skip]", - warning: "[warn]" - }[status]; - } - return { - blocked: "x", - info: "i", - passed: "\u2713", - skipped: "-", - warning: "!" - }[status]; + return (supportsUnicode(options) ? UNICODE_STATUS_SYMBOLS : ASCII_STATUS_SYMBOLS)[status]; } function styleStatus(value, status, options) { - switch (status) { - case "blocked": - return style(value, "red", options); - case "info": - return style(value, "blue", options); - case "passed": - return style(value, "green", options); - case "skipped": - return style(value, "dim", options); - case "warning": - return style(value, "yellow", options); - } + return style(value, STATUS_COLORS[status], options); } function style(value, color, options) { if (!supportsColor(options)) { @@ -10260,10 +10262,19 @@ function writeGitPushSuccessSummary(stream, summary, options = {}) { } function parseGitPushArgs(args) { const positionals = []; + let remoteFromOption; + let hasRemoteFromOption = false; let parseOptions = true; let setsUpstream = false; + let readNextAsRemote = false; let skipNext = false; for (const arg of args) { + if (readNextAsRemote) { + remoteFromOption = arg || void 0; + hasRemoteFromOption = true; + readNextAsRemote = false; + continue; + } if (skipNext) { skipNext = false; continue; @@ -10277,19 +10288,37 @@ function parseGitPushArgs(args) { continue; } if (parseOptions && arg.startsWith("-")) { + const inlineRemote = inlineOptionValue(arg, "--repo"); + if (inlineRemote !== void 0) { + remoteFromOption = inlineRemote || void 0; + hasRemoteFromOption = true; + continue; + } + if (arg === "--repo") { + readNextAsRemote = true; + continue; + } skipNext = optionTakesSeparateValue(arg); continue; } positionals.push(arg); } + const branchPosition = hasRemoteFromOption ? 0 : 1; return { - branch: positionals[1] ? branchFromRefspec(positionals[1]) : void 0, - remote: positionals[0], + branch: positionals[branchPosition] ? branchFromRefspec(positionals[branchPosition]) : void 0, + remote: hasRemoteFromOption ? remoteFromOption : positionals[0], setsUpstream }; } +function inlineOptionValue(arg, option) { + const prefix = `${option}=`; + return arg.startsWith(prefix) ? arg.slice(prefix.length) : void 0; +} function optionTakesSeparateValue(arg) { - return arg === "--exec" || arg === "--receive-pack" || arg === "--repo" || arg === "--push-option" || arg === "-o"; + if (arg.includes("=")) { + return false; + } + return arg === "--exec" || arg === "--recurse-submodules" || arg === "--receive-pack" || arg === "--push-option" || arg === "-o"; } function branchFromRefspec(refspec) { let branch = refspec.trim(); diff --git a/hook/pre-push b/hook/pre-push index a2eea1c..c2c8bad 100755 --- a/hook/pre-push +++ b/hook/pre-push @@ -87,6 +87,29 @@ resolve_runner() { return 1 } +runner_uses_node() { + local first_line + + IFS= read -r first_line < "$PUSHGATE_RUNNER" || first_line="" + case "$first_line" in + '#!'*node*) + return 0 + ;; + *) + return 1 + ;; + esac +} + +run_runner() { + if runner_uses_node; then + node "$PUSHGATE_RUNNER" "$@" + return $? + fi + + "$PUSHGATE_RUNNER" "$@" +} + REPO_ROOT="$(repo_root)" if ! resolve_runner; then @@ -107,7 +130,7 @@ if [ ! -x "$PUSHGATE_RUNNER" ]; then exit 1 fi -if ! RUNNER_PROTOCOL="$("$PUSHGATE_RUNNER" hook-protocol 2>&1)"; then +if ! RUNNER_PROTOCOL="$(run_runner hook-protocol 2>&1)"; then error "Pushgate runner from ${PUSHGATE_RUNNER_SOURCE} at ${PUSHGATE_RUNNER} could not report its hook protocol." if [ -n "$RUNNER_PROTOCOL" ]; then error "Runner output:" @@ -130,4 +153,8 @@ fi verbose "Pushgate pre-push hook v${HOOK_VERSION} with protocol ${HOOK_PROTOCOL}" verbose "Repository root: ${REPO_ROOT}" verbose "Using runner from ${PUSHGATE_RUNNER_SOURCE}: ${PUSHGATE_RUNNER}" +if runner_uses_node; then + exec node "$PUSHGATE_RUNNER" pre-push "$@" +fi + exec "$PUSHGATE_RUNNER" pre-push "$@" diff --git a/src/git/push.ts b/src/git/push.ts index 85c217d..23195d1 100644 --- a/src/git/push.ts +++ b/src/git/push.ts @@ -92,11 +92,21 @@ export function writeGitPushSuccessSummary( function parseGitPushArgs(args: readonly string[]): ParsedGitPushArgs { const positionals: string[] = []; + let remoteFromOption: string | undefined; + let hasRemoteFromOption = false; let parseOptions = true; let setsUpstream = false; + let readNextAsRemote = false; let skipNext = false; for (const arg of args) { + if (readNextAsRemote) { + remoteFromOption = arg || undefined; + hasRemoteFromOption = true; + readNextAsRemote = false; + continue; + } + if (skipNext) { skipNext = false; continue; @@ -113,6 +123,19 @@ function parseGitPushArgs(args: readonly string[]): ParsedGitPushArgs { } if (parseOptions && arg.startsWith("-")) { + const inlineRemote = inlineOptionValue(arg, "--repo"); + + if (inlineRemote !== undefined) { + remoteFromOption = inlineRemote || undefined; + hasRemoteFromOption = true; + continue; + } + + if (arg === "--repo") { + readNextAsRemote = true; + continue; + } + skipNext = optionTakesSeparateValue(arg); continue; } @@ -120,18 +143,32 @@ function parseGitPushArgs(args: readonly string[]): ParsedGitPushArgs { positionals.push(arg); } + const branchPosition = hasRemoteFromOption ? 0 : 1; + return { - branch: positionals[1] ? branchFromRefspec(positionals[1]) : undefined, - remote: positionals[0], + branch: positionals[branchPosition] + ? branchFromRefspec(positionals[branchPosition]) + : undefined, + remote: hasRemoteFromOption ? remoteFromOption : positionals[0], setsUpstream, }; } +function inlineOptionValue(arg: string, option: string): string | undefined { + const prefix = `${option}=`; + + return arg.startsWith(prefix) ? arg.slice(prefix.length) : undefined; +} + function optionTakesSeparateValue(arg: string): boolean { + if (arg.includes("=")) { + return false; + } + return ( arg === "--exec" || + arg === "--recurse-submodules" || arg === "--receive-pack" || - arg === "--repo" || arg === "--push-option" || arg === "-o" ); diff --git a/src/process/captured-command.ts b/src/process/captured-command.ts index 3de7665..2ee1ea4 100644 --- a/src/process/captured-command.ts +++ b/src/process/captured-command.ts @@ -63,7 +63,6 @@ export function runCapturedCommand( const stdoutBuffers: Buffer[] = []; let stdout = ""; let stderr = ""; - let exited = false; let timedOut = false; let settled = false; let killTimer: NodeJS.Timeout | undefined; @@ -84,6 +83,14 @@ export function runCapturedCommand( outputEncoding === "utf8" && options.outputTailLimit !== undefined ? formatOutputTail(stdout, stderr, options.outputTailLimit) : undefined; + const finishTimeout = () => { + finish({ + kind: "timeout", + outputTail: capturedOutputTail(), + stderr, + stdout: capturedStdout(), + }); + }; const finish = (result: CapturedCommandResult) => { if (settled) { return; @@ -106,9 +113,8 @@ export function runCapturedCommand( timedOut = true; signalChild("SIGTERM"); killTimer = setTimeout(() => { - if (useProcessGroup || !exited) { - signalChild("SIGKILL"); - } + signalChild("SIGKILL"); + killTimer = undefined; }, options.killGraceMs ?? 0); }, options.timeoutMs); } @@ -149,8 +155,6 @@ export function runCapturedCommand( }); }); child.on("exit", () => { - exited = true; - if (!useProcessGroup && killTimer) { clearTimeout(killTimer); killTimer = undefined; @@ -158,15 +162,20 @@ export function runCapturedCommand( }); child.on("close", (code, signal) => { if (timedOut) { - finish({ - kind: "timeout", - outputTail: capturedOutputTail(), - stderr, - stdout: capturedStdout(), - }); + if (useProcessGroup && killTimer) { + clearTimeout(killTimer); + killTimer = undefined; + signalChild("SIGKILL"); + } + + finishTimeout(); return; } + if (useProcessGroup) { + signalChild("SIGKILL"); + } + finish({ code, kind: "completed", diff --git a/src/terminal/format.ts b/src/terminal/format.ts index 75813f1..f0df871 100644 --- a/src/terminal/format.ts +++ b/src/terminal/format.ts @@ -14,6 +14,30 @@ const ANSI = { yellow: ["\u001B[33m", "\u001B[39m"], } as const; +const ASCII_STATUS_SYMBOLS = { + blocked: "[block]", + info: "[info]", + passed: "[ok]", + skipped: "[skip]", + warning: "[warn]", +} as const satisfies Record; + +const UNICODE_STATUS_SYMBOLS = { + blocked: "x", + info: "i", + passed: "\u2713", + skipped: "-", + warning: "!", +} as const satisfies Record; + +const STATUS_COLORS = { + blocked: "red", + info: "blue", + passed: "green", + skipped: "dim", + warning: "yellow", +} as const satisfies Record; + const LABEL_WIDTH = 18; export function writeHeader( @@ -115,25 +139,9 @@ function statusSymbol( status: TerminalStatus, options: TerminalStyleOptions, ): string { - const unicode = supportsUnicode(options); - - if (!unicode) { - return { - blocked: "[block]", - info: "[info]", - passed: "[ok]", - skipped: "[skip]", - warning: "[warn]", - }[status]; - } - - return { - blocked: "x", - info: "i", - passed: "\u2713", - skipped: "-", - warning: "!", - }[status]; + return (supportsUnicode(options) ? UNICODE_STATUS_SYMBOLS : ASCII_STATUS_SYMBOLS)[ + status + ]; } function styleStatus( @@ -141,18 +149,7 @@ function styleStatus( status: TerminalStatus, options: TerminalStyleOptions, ): string { - switch (status) { - case "blocked": - return style(value, "red", options); - case "info": - return style(value, "blue", options); - case "passed": - return style(value, "green", options); - case "skipped": - return style(value, "dim", options); - case "warning": - return style(value, "yellow", options); - } + return style(value, STATUS_COLORS[status], options); } function style( diff --git a/test/deterministic-runner.test.ts b/test/deterministic-runner.test.ts index b86cbd6..80d7f8f 100644 --- a/test/deterministic-runner.test.ts +++ b/test/deterministic-runner.test.ts @@ -573,6 +573,11 @@ test("renders deterministic transcript without running commands", () => { status: "passed", detail: "5 changed line(s) within max_changed_lines 10", }); + transcript.writePolicyResult({ + name: "policy:diff_size", + status: "passed", + detail: "", + }); transcript.writeToolResult(tool(), { name: "check", status: "blocked", @@ -592,6 +597,7 @@ test("renders deterministic transcript without running commands", () => { "Checks", " Running 3 checks.", " [ok] Diff size 5 / 10 changed lines", + " [ok] Diff size", " [block] Check exited with code 2", " Command output:", " first line", diff --git a/test/install.test.ts b/test/install.test.ts index 2988ecd..1911df0 100644 --- a/test/install.test.ts +++ b/test/install.test.ts @@ -84,7 +84,7 @@ test("installs the managed runner, thin hook backup, and v2 config", async () => assert.match(await readFile(runnerPath, "utf8"), /HOOK_PROTOCOL = "1"/); assert.match( await readFile(join(harness.hooksDir, "pre-push"), "utf8"), - /exec "\$PUSHGATE_RUNNER" pre-push "\$@"/, + /exec node "\$PUSHGATE_RUNNER" pre-push "\$@"/, ); const backups = (await readdir(harness.hooksDir)).filter((name) => diff --git a/test/process.test.ts b/test/process.test.ts index ded1eb0..fbf1425 100644 --- a/test/process.test.ts +++ b/test/process.test.ts @@ -1,4 +1,6 @@ import assert from "node:assert/strict"; +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; import { join } from "node:path"; import test from "node:test"; @@ -101,25 +103,18 @@ test("runTimedCommand reports timeout with captured output tail", async () => { } }); -test("runTimedCommand terminates timed-out process groups", async () => { +test("runTimedCommand clears stale kill timers after SIGTERM exits", async () => { if (process.platform === "win32") { return; } const started = Date.now(); const result = await runTimedCommand({ - args: [ - "-e", - [ - "const { spawn } = require('node:child_process');", - "spawn(process.execPath, ['-e', 'setTimeout(() => {}, 5000);'], { stdio: ['ignore', 'inherit', 'inherit'] });", - "setInterval(() => {}, 1000);", - ].join(" "), - ], + args: ["-e", "setInterval(() => {}, 1000);"], command: process.execPath, cwd: process.cwd(), env: process.env, - killGraceMs: 50, + killGraceMs: 4_000, timeoutSeconds: 1, }); const elapsedMs = Date.now() - started; @@ -127,10 +122,101 @@ test("runTimedCommand terminates timed-out process groups", async () => { assert.equal(result.kind, "timeout"); assert.ok( elapsedMs < 3_000, - `expected process group cleanup before grandchild sleep finished, took ${elapsedMs}ms`, + `expected timeout to settle after SIGTERM, took ${elapsedMs}ms`, ); }); +test("runTimedCommand cleans up background descendants after completion", async () => { + if (process.platform === "win32") { + return; + } + + const tempRoot = await mkdtemp(join(tmpdir(), "pushgate-process-")); + const grandchildPidPath = join(tempRoot, "grandchild.pid"); + const grandchildScript = [ + "const { writeFileSync } = require('node:fs');", + "writeFileSync(process.argv[1], String(process.pid));", + "process.on('SIGTERM', () => {});", + "setInterval(() => {}, 1000);", + ].join(" "); + const parentScript = [ + "const { existsSync } = require('node:fs');", + "const { spawn } = require('node:child_process');", + "const grandchild = spawn(process.execPath, ['-e', process.argv[2], process.argv[1]], { stdio: ['ignore', 'ignore', 'ignore'] });", + "grandchild.unref();", + "const started = Date.now();", + "while (!existsSync(process.argv[1]) && Date.now() - started < 1000) {}", + ].join(" "); + let grandchildPid: number | undefined; + + try { + const result = await runTimedCommand({ + args: ["-e", parentScript, grandchildPidPath, grandchildScript], + command: process.execPath, + cwd: process.cwd(), + env: process.env, + timeoutSeconds: 10, + }); + grandchildPid = Number(await readFile(grandchildPidPath, "utf8")); + + assert.equal(result.kind, "completed"); + if (result.kind === "completed") { + assert.equal(result.code, 0); + } + await assertProcessIsGone(grandchildPid); + } finally { + if (grandchildPid !== undefined) { + killProcessIfRunning(grandchildPid); + } + + await rm(tempRoot, { force: true, recursive: true }); + } +}); + +test("runTimedCommand terminates timed-out process groups", async () => { + if (process.platform === "win32") { + return; + } + + const tempRoot = await mkdtemp(join(tmpdir(), "pushgate-process-")); + const grandchildPidPath = join(tempRoot, "grandchild.pid"); + const grandchildScript = [ + "const { writeFileSync } = require('node:fs');", + "writeFileSync(process.argv[1], String(process.pid));", + "process.on('SIGTERM', () => {});", + "setInterval(() => {}, 1000);", + ].join(" "); + const parentScript = [ + "const { spawn } = require('node:child_process');", + "const grandchild = spawn(process.execPath, ['-e', process.argv[2], process.argv[1]], { stdio: ['ignore', 'ignore', 'ignore'] });", + "grandchild.unref();", + "setInterval(() => {}, 1000);", + ].join(" "); + const started = Date.now(); + + try { + const result = await runTimedCommand({ + args: ["-e", parentScript, grandchildPidPath, grandchildScript], + command: process.execPath, + cwd: process.cwd(), + env: process.env, + killGraceMs: 4_000, + timeoutSeconds: 1, + }); + const elapsedMs = Date.now() - started; + const grandchildPid = Number(await readFile(grandchildPidPath, "utf8")); + + assert.equal(result.kind, "timeout"); + assert.ok( + elapsedMs < 3_000, + `expected process group cleanup before grandchild sleep finished, took ${elapsedMs}ms`, + ); + await assertProcessIsGone(grandchildPid); + } finally { + await rm(tempRoot, { force: true, recursive: true }); + } +}); + test("runTimedCommand stdin broken pipes do not override close results", async () => { const result = await runTimedCommand({ args: ["-e", "process.exit(0);"], @@ -262,3 +348,52 @@ test("process outcome policy classifies failures and formats diagnostics", async ); assert.equal(formatProcessFailure(timeout.failure), "timed out after 1s"); }); + +async function assertProcessIsGone( + pid: number, + timeoutMs = 2_000, +): Promise { + const started = Date.now(); + + while (Date.now() - started < timeoutMs) { + if (!processIsRunning(pid)) { + return; + } + + await new Promise((resolve) => setTimeout(resolve, 50)); + } + + assert.fail(`expected process ${String(pid)} to be terminated`); +} + +function processIsRunning(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + if (isMissingProcessError(error)) { + return false; + } + + throw error; + } +} + +function killProcessIfRunning(pid: number): void { + try { + process.kill(pid, "SIGKILL"); + } catch (error) { + if (!isMissingProcessError(error)) { + throw error; + } + } +} + +function isMissingProcessError(error: unknown): boolean { + return ( + error !== null && + typeof error === "object" && + "code" in error && + (error as { code?: unknown }).code === "ESRCH" + ); +} diff --git a/test/push.test.ts b/test/push.test.ts index 8446371..561828a 100644 --- a/test/push.test.ts +++ b/test/push.test.ts @@ -1,12 +1,20 @@ import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { Writable } from "node:stream"; import test from "node:test"; +import { promisify } from "node:util"; import { branchFromRefspec, + resolveGitPushSuccessSummary, writeGitPushSuccessSummary, } from "../src/git/push.js"; +const execFileAsync = promisify(execFile); + test("writeGitPushSuccessSummary uses shared terminal result formatting", () => { const output = captureOutput(); @@ -65,6 +73,83 @@ test("branchFromRefspec ignores refspecs that cannot name a pushed branch", () = assert.equal(branchFromRefspec("refs/tags/v1.0.0"), undefined); }); +test("resolveGitPushSuccessSummary parses repository options from push args", async () => { + await withGitRemote( + "git@github.com:rootstrap/ai-pushgate.git", + async (env) => { + assert.deepEqual( + await resolveGitPushSuccessSummary( + ["--receive-pack=git-receive-pack", "--repo=origin", "feature"], + { env }, + ), + { + pullRequestUrl: + "https://github.com/rootstrap/ai-pushgate/pull/new/feature", + upstream: undefined, + }, + ); + + assert.deepEqual( + await resolveGitPushSuccessSummary( + ["--repo", "origin", "refs/heads/feature:refs/heads/release"], + { env }, + ), + { + pullRequestUrl: + "https://github.com/rootstrap/ai-pushgate/pull/new/release", + upstream: undefined, + }, + ); + + assert.deepEqual( + await resolveGitPushSuccessSummary( + ["--set-upstream", "--repo=origin", "feature"], + { env }, + ), + { + pullRequestUrl: + "https://github.com/rootstrap/ai-pushgate/pull/new/feature", + upstream: "origin/feature", + }, + ); + }, + ); +}); + +test("resolveGitPushSuccessSummary ignores remotes that cannot become GitHub pull request URLs", async () => { + await withGitRemote( + "https://gitlab.com/rootstrap/ai-pushgate.git", + async (env) => { + assert.deepEqual( + await resolveGitPushSuccessSummary(["origin", "feature"], { env }), + { pullRequestUrl: undefined, upstream: undefined }, + ); + }, + ); + + await withGitRemote("not a url", async (env) => { + assert.deepEqual( + await resolveGitPushSuccessSummary(["origin", "feature"], { env }), + { pullRequestUrl: undefined, upstream: undefined }, + ); + }); +}); + +test("resolveGitPushSuccessSummary treats git command failures as best effort misses", async () => { + const env = { + ...process.env, + PATH: join( + tmpdir(), + `.missing-pushgate-git-path-${String(process.pid)}`, + ), + }; + + assert.deepEqual( + await resolveGitPushSuccessSummary(["origin", "feature"], { env }), + { pullRequestUrl: undefined, upstream: undefined }, + ); +}); + function captureOutput(): { stream: Writable; text(): string; @@ -84,3 +169,25 @@ function captureOutput(): { }, }; } + +async function withGitRemote( + remoteUrl: string, + callback: (env: NodeJS.ProcessEnv) => Promise, +): Promise { + const repoRoot = await mkdtemp(join(tmpdir(), "pushgate-push-")); + + try { + await execFileAsync("git", ["init", "--quiet"], { cwd: repoRoot }); + await execFileAsync("git", ["config", "remote.origin.url", remoteUrl], { + cwd: repoRoot, + }); + + await callback({ + ...process.env, + GIT_DIR: join(repoRoot, ".git"), + GIT_WORK_TREE: repoRoot, + }); + } finally { + await rm(repoRoot, { force: true, recursive: true }); + } +} From 65f2036e18ace59111eb1c46d27bbb24a333b3b0 Mon Sep 17 00:00:00 2001 From: Pushgate Hook Harness Date: Sat, 27 Jun 2026 17:20:24 -0300 Subject: [PATCH 5/5] Enhance Pushgate CLI to support non-branch push intents and improve command handling --- bin/pushgate.mjs | 97 ++++++++++++++------ src/git/push.ts | 54 ++++++++++-- src/process/captured-command.ts | 16 ++-- src/workflows/pre-push.ts | 78 +++++++++++++---- test/pre-push-stdin.test.ts | 48 ++++++++++ test/process.test.ts | 21 ++++- test/push.test.ts | 151 +++++++++++++++++++++++++++++++- 7 files changed, 395 insertions(+), 70 deletions(-) create mode 100644 test/pre-push-stdin.test.ts diff --git a/bin/pushgate.mjs b/bin/pushgate.mjs index 2e94d31..2eed70b 100755 --- a/bin/pushgate.mjs +++ b/bin/pushgate.mjs @@ -9613,24 +9613,19 @@ function runCapturedCommand(options) { }); }); child.on("exit", () => { - if (!useProcessGroup && killTimer) { + if (killTimer) { clearTimeout(killTimer); killTimer = void 0; + if (useProcessGroup && timedOut) { + signalChild("SIGKILL"); + } } }); child.on("close", (code, signal) => { if (timedOut) { - if (useProcessGroup && killTimer) { - clearTimeout(killTimer); - killTimer = void 0; - signalChild("SIGKILL"); - } finishTimeout(); return; } - if (useProcessGroup) { - signalChild("SIGKILL"); - } finish({ code, kind: "completed", @@ -10237,17 +10232,24 @@ function runGitPush(args, options) { } async function resolveGitPushSuccessSummary(args, options) { const parsed = parseGitPushArgs(args); + if (parsed.intent === "non-branch") { + return { kind: "non-branch" }; + } const currentUpstream = parsed.setsUpstream ? await readCurrentUpstream(options.env) : void 0; const branch = parsed.branch ?? branchFromUpstream(currentUpstream) ?? await readCurrentBranch(options.env); const remote = parsed.remote ?? remoteFromUpstream(currentUpstream) ?? (branch ? await readConfiguredBranchRemote(branch, options.env) : void 0); const upstream = parsed.setsUpstream ? currentUpstream ?? upstreamFromParts(remote, branch) : void 0; const remoteUrl = remote ? await readRemoteUrl(remote, options.env) : void 0; return { + kind: "branch-update", pullRequestUrl: remoteUrl && branch ? githubPullRequestUrl(remoteUrl, branch) : void 0, upstream }; } function writeGitPushSuccessSummary(stream, summary, options = {}) { + if (summary.kind === "non-branch") { + return; + } writeLine(stream); writeSection(stream, "Pushing branch", options); writeResultRow(stream, "passed", "Branch pushed", void 0, options); @@ -10265,6 +10267,7 @@ function parseGitPushArgs(args) { let remoteFromOption; let hasRemoteFromOption = false; let parseOptions = true; + let nonBranchPush = false; let setsUpstream = false; let readNextAsRemote = false; let skipNext = false; @@ -10288,6 +10291,10 @@ function parseGitPushArgs(args) { continue; } if (parseOptions && arg.startsWith("-")) { + if (isNonBranchPushOption(arg)) { + nonBranchPush = true; + continue; + } const inlineRemote = inlineOptionValue(arg, "--repo"); if (inlineRemote !== void 0) { remoteFromOption = inlineRemote || void 0; @@ -10304,8 +10311,11 @@ function parseGitPushArgs(args) { positionals.push(arg); } const branchPosition = hasRemoteFromOption ? 0 : 1; + const refspec = positionals[branchPosition]; + const branch = refspec ? branchFromRefspec(refspec) : void 0; return { - branch: positionals[branchPosition] ? branchFromRefspec(positionals[branchPosition]) : void 0, + branch, + intent: nonBranchPush || refspec !== void 0 && branch === void 0 ? "non-branch" : "branch-update", remote: hasRemoteFromOption ? remoteFromOption : positionals[0], setsUpstream }; @@ -10320,6 +10330,9 @@ function optionTakesSeparateValue(arg) { } return arg === "--exec" || arg === "--recurse-submodules" || arg === "--receive-pack" || arg === "--push-option" || arg === "-o"; } +function isNonBranchPushOption(arg) { + return arg === "--all" || arg === "--delete" || arg === "-d" || arg === "--mirror" || arg === "--tags"; +} function branchFromRefspec(refspec) { let branch = refspec.trim(); if (!branch || branch.includes("*")) { @@ -27953,7 +27966,7 @@ function createTerminalWarningConfirmer(options = {}) { async function runPrePushWorkflow(io) { const hookContext = buildPrePushContext({ args: io.hookArgs ?? [], - stdin: await readStdin(io.stdin) + branch: await readPrePushBranchFromStdin(io.stdin) }); const repoRoot = await resolveGitRepositoryRoot(io.env); writePrePushHeader(io.stdout, repoRoot, hookContext); @@ -28113,37 +28126,67 @@ function writePrePushHeader(stdout, repoRoot, context) { } function buildPrePushContext(options) { return { - branch: parseBranchFromPrePushInput(options.stdin), + branch: options.branch, remote: options.args[0] }; } -function parseBranchFromPrePushInput(input) { - for (const line of input.split(/\r?\n/)) { - const trimmed = line.trim(); - if (!trimmed) { - continue; - } - const [localRef] = trimmed.split(/\s+/, 1); - if (localRef?.startsWith("refs/heads/")) { - return localRef.slice("refs/heads/".length); - } +var MAX_PRE_PUSH_STDIN_LINE_CHARS = 8 * 1024; +function parseBranchFromPrePushLine(line) { + const trimmed = line.trim(); + if (!trimmed) { + return void 0; + } + const [localRef] = trimmed.split(/\s+/, 1); + if (localRef?.startsWith("refs/heads/")) { + return localRef.slice("refs/heads/".length); } return void 0; } -function readStdin(stdin) { +function readPrePushBranchFromStdin(stdin) { return new Promise((resolve, reject) => { if (stdin.isTTY) { - resolve(""); + resolve(void 0); return; } - let input = ""; + let branch; + let line = ""; + let lineOverflowed = false; + const parseLine = () => { + if (branch !== void 0 || lineOverflowed) { + return; + } + branch = parseBranchFromPrePushLine(line); + }; stdin.setEncoding("utf8"); stdin.on("error", reject); stdin.on("data", (chunk) => { - input += chunk; + if (branch !== void 0) { + return; + } + for (const character of chunk) { + if (character === "\n") { + if (line.endsWith("\r")) { + line = line.slice(0, -1); + } + parseLine(); + line = ""; + lineOverflowed = false; + continue; + } + if (lineOverflowed) { + continue; + } + if (line.length >= MAX_PRE_PUSH_STDIN_LINE_CHARS) { + line = ""; + lineOverflowed = true; + continue; + } + line += character; + } }); stdin.on("end", () => { - resolve(input); + parseLine(); + resolve(branch); }); stdin.resume(); }); diff --git a/src/git/push.ts b/src/git/push.ts index 23195d1..05a96c3 100644 --- a/src/git/push.ts +++ b/src/git/push.ts @@ -12,13 +12,19 @@ export interface GitPushResult { signal: NodeJS.Signals | null; } -export interface GitPushSuccessSummary { - pullRequestUrl?: string; - upstream?: string; -} +export type GitPushSuccessSummary = + | { + kind: "branch-update"; + pullRequestUrl?: string; + upstream?: string; + } + | { + kind: "non-branch"; + }; -interface ParsedGitPushArgs { +export interface ParsedGitPushArgs { branch?: string; + intent: "branch-update" | "non-branch"; remote?: string; setsUpstream: boolean; } @@ -43,6 +49,11 @@ export async function resolveGitPushSuccessSummary( }, ): Promise { const parsed = parseGitPushArgs(args); + + if (parsed.intent === "non-branch") { + return { kind: "non-branch" }; + } + const currentUpstream = parsed.setsUpstream ? await readCurrentUpstream(options.env) : undefined; @@ -60,6 +71,7 @@ export async function resolveGitPushSuccessSummary( const remoteUrl = remote ? await readRemoteUrl(remote, options.env) : undefined; return { + kind: "branch-update", pullRequestUrl: remoteUrl && branch ? githubPullRequestUrl(remoteUrl, branch) @@ -75,6 +87,10 @@ export function writeGitPushSuccessSummary( env?: NodeJS.ProcessEnv; } = {}, ): void { + if (summary.kind === "non-branch") { + return; + } + writeLine(stream); writeSection(stream, "Pushing branch", options); writeResultRow(stream, "passed", "Branch pushed", undefined, options); @@ -90,11 +106,12 @@ export function writeGitPushSuccessSummary( } } -function parseGitPushArgs(args: readonly string[]): ParsedGitPushArgs { +export function parseGitPushArgs(args: readonly string[]): ParsedGitPushArgs { const positionals: string[] = []; let remoteFromOption: string | undefined; let hasRemoteFromOption = false; let parseOptions = true; + let nonBranchPush = false; let setsUpstream = false; let readNextAsRemote = false; let skipNext = false; @@ -123,6 +140,11 @@ function parseGitPushArgs(args: readonly string[]): ParsedGitPushArgs { } if (parseOptions && arg.startsWith("-")) { + if (isNonBranchPushOption(arg)) { + nonBranchPush = true; + continue; + } + const inlineRemote = inlineOptionValue(arg, "--repo"); if (inlineRemote !== undefined) { @@ -144,11 +166,15 @@ function parseGitPushArgs(args: readonly string[]): ParsedGitPushArgs { } const branchPosition = hasRemoteFromOption ? 0 : 1; + const refspec = positionals[branchPosition]; + const branch = refspec ? branchFromRefspec(refspec) : undefined; return { - branch: positionals[branchPosition] - ? branchFromRefspec(positionals[branchPosition]) - : undefined, + branch, + intent: + nonBranchPush || (refspec !== undefined && branch === undefined) + ? "non-branch" + : "branch-update", remote: hasRemoteFromOption ? remoteFromOption : positionals[0], setsUpstream, }; @@ -174,6 +200,16 @@ function optionTakesSeparateValue(arg: string): boolean { ); } +function isNonBranchPushOption(arg: string): boolean { + return ( + arg === "--all" || + arg === "--delete" || + arg === "-d" || + arg === "--mirror" || + arg === "--tags" + ); +} + export function branchFromRefspec(refspec: string): string | undefined { let branch = refspec.trim(); diff --git a/src/process/captured-command.ts b/src/process/captured-command.ts index 2ee1ea4..d4fd47b 100644 --- a/src/process/captured-command.ts +++ b/src/process/captured-command.ts @@ -155,27 +155,21 @@ export function runCapturedCommand( }); }); child.on("exit", () => { - if (!useProcessGroup && killTimer) { + if (killTimer) { clearTimeout(killTimer); killTimer = undefined; + + if (useProcessGroup && timedOut) { + signalChild("SIGKILL"); + } } }); child.on("close", (code, signal) => { if (timedOut) { - if (useProcessGroup && killTimer) { - clearTimeout(killTimer); - killTimer = undefined; - signalChild("SIGKILL"); - } - finishTimeout(); return; } - if (useProcessGroup) { - signalChild("SIGKILL"); - } - finish({ code, kind: "completed", diff --git a/src/workflows/pre-push.ts b/src/workflows/pre-push.ts index 133f86d..38c3d5e 100644 --- a/src/workflows/pre-push.ts +++ b/src/workflows/pre-push.ts @@ -46,7 +46,7 @@ export async function runPrePushWorkflow( ): Promise { const hookContext = buildPrePushContext({ args: io.hookArgs ?? [], - stdin: await readStdin(io.stdin), + branch: await readPrePushBranchFromStdin(io.stdin), }); const repoRoot = await resolveGitRepositoryRoot(io.env); @@ -276,48 +276,90 @@ function writePrePushHeader( function buildPrePushContext(options: { args: readonly string[]; - stdin: string; + branch: string | undefined; }): PrePushContext { return { - branch: parseBranchFromPrePushInput(options.stdin), + branch: options.branch, remote: options.args[0], }; } -function parseBranchFromPrePushInput(input: string): string | undefined { - for (const line of input.split(/\r?\n/)) { - const trimmed = line.trim(); +const MAX_PRE_PUSH_STDIN_LINE_CHARS = 8 * 1024; - if (!trimmed) { - continue; - } +export function parseBranchFromPrePushLine( + line: string, +): string | undefined { + const trimmed = line.trim(); - const [localRef] = trimmed.split(/\s+/, 1); + if (!trimmed) { + return undefined; + } - if (localRef?.startsWith("refs/heads/")) { - return localRef.slice("refs/heads/".length); - } + const [localRef] = trimmed.split(/\s+/, 1); + + if (localRef?.startsWith("refs/heads/")) { + return localRef.slice("refs/heads/".length); } return undefined; } -function readStdin(stdin: NodeJS.ReadableStream): Promise { +export function readPrePushBranchFromStdin( + stdin: NodeJS.ReadableStream, +): Promise { return new Promise((resolve, reject) => { if ((stdin as { isTTY?: boolean }).isTTY) { - resolve(""); + resolve(undefined); return; } - let input = ""; + let branch: string | undefined; + let line = ""; + let lineOverflowed = false; + + const parseLine = () => { + if (branch !== undefined || lineOverflowed) { + return; + } + + branch = parseBranchFromPrePushLine(line); + }; stdin.setEncoding("utf8"); stdin.on("error", reject); stdin.on("data", (chunk: string) => { - input += chunk; + if (branch !== undefined) { + return; + } + + for (const character of chunk) { + if (character === "\n") { + if (line.endsWith("\r")) { + line = line.slice(0, -1); + } + + parseLine(); + line = ""; + lineOverflowed = false; + continue; + } + + if (lineOverflowed) { + continue; + } + + if (line.length >= MAX_PRE_PUSH_STDIN_LINE_CHARS) { + line = ""; + lineOverflowed = true; + continue; + } + + line += character; + } }); stdin.on("end", () => { - resolve(input); + parseLine(); + resolve(branch); }); stdin.resume(); }); diff --git a/test/pre-push-stdin.test.ts b/test/pre-push-stdin.test.ts new file mode 100644 index 0000000..d6c1bc1 --- /dev/null +++ b/test/pre-push-stdin.test.ts @@ -0,0 +1,48 @@ +import assert from "node:assert/strict"; +import { Readable } from "node:stream"; +import test from "node:test"; + +import { + parseBranchFromPrePushLine, + readPrePushBranchFromStdin, +} from "../src/workflows/pre-push.js"; + +test("parseBranchFromPrePushLine returns the local branch ref", () => { + assert.equal( + parseBranchFromPrePushLine( + "refs/heads/feature local-sha refs/heads/feature remote-sha", + ), + "feature", + ); + assert.equal( + parseBranchFromPrePushLine( + "refs/tags/v1.0.0 local-sha refs/tags/v1.0.0 remote-sha", + ), + undefined, + ); + assert.equal(parseBranchFromPrePushLine(""), undefined); +}); + +test("readPrePushBranchFromStdin parses incrementally and discards trailing input", async () => { + const branch = await readPrePushBranchFromStdin( + Readable.from([ + "refs/tags/v1.0.0 local-sha refs/tags/v1.0.0 remote-sha\n", + "refs/heads/feature local-sha refs/heads/feature remote-sha\n", + "x".repeat(2 * 1024 * 1024), + ]), + ); + + assert.equal(branch, "feature"); +}); + +test("readPrePushBranchFromStdin skips oversized lines without buffering them", async () => { + const branch = await readPrePushBranchFromStdin( + Readable.from([ + "x".repeat(128 * 1024), + "\n", + "refs/heads/main local-sha refs/heads/main remote-sha\n", + ]), + ); + + assert.equal(branch, "main"); +}); diff --git a/test/process.test.ts b/test/process.test.ts index fbf1425..693a8cb 100644 --- a/test/process.test.ts +++ b/test/process.test.ts @@ -126,7 +126,7 @@ test("runTimedCommand clears stale kill timers after SIGTERM exits", async () => ); }); -test("runTimedCommand cleans up background descendants after completion", async () => { +test("runTimedCommand does not kill successful process groups after close", async () => { if (process.platform === "win32") { return; } @@ -163,7 +163,7 @@ test("runTimedCommand cleans up background descendants after completion", async if (result.kind === "completed") { assert.equal(result.code, 0); } - await assertProcessIsGone(grandchildPid); + await assertProcessIsRunning(grandchildPid); } finally { if (grandchildPid !== undefined) { killProcessIfRunning(grandchildPid); @@ -366,6 +366,23 @@ async function assertProcessIsGone( assert.fail(`expected process ${String(pid)} to be terminated`); } +async function assertProcessIsRunning( + pid: number, + timeoutMs = 1_000, +): Promise { + const started = Date.now(); + + while (Date.now() - started < timeoutMs) { + if (processIsRunning(pid)) { + return; + } + + await new Promise((resolve) => setTimeout(resolve, 50)); + } + + assert.fail(`expected process ${String(pid)} to still be running`); +} + function processIsRunning(pid: number): boolean { try { process.kill(pid, 0); diff --git a/test/push.test.ts b/test/push.test.ts index 561828a..aa59b21 100644 --- a/test/push.test.ts +++ b/test/push.test.ts @@ -9,6 +9,7 @@ import { promisify } from "node:util"; import { branchFromRefspec, + parseGitPushArgs, resolveGitPushSuccessSummary, writeGitPushSuccessSummary, } from "../src/git/push.js"; @@ -26,6 +27,7 @@ test("writeGitPushSuccessSummary uses shared terminal result formatting", () => writeGitPushSuccessSummary( output.stream, { + kind: "branch-update", pullRequestUrl: "https://github.com/rootstrap/ai-pushgate/pull/new/main", upstream: "origin/main", }, @@ -50,6 +52,14 @@ test("writeGitPushSuccessSummary uses shared terminal result formatting", () => ); }); +test("writeGitPushSuccessSummary stays quiet for non-branch pushes", () => { + const output = captureOutput(); + + writeGitPushSuccessSummary(output.stream, { kind: "non-branch" }); + + assert.equal(output.text(), ""); +}); + test("branchFromRefspec returns the pushed destination branch", () => { assert.equal(branchFromRefspec("feature"), "feature"); assert.equal(branchFromRefspec("+feature"), "feature"); @@ -73,6 +83,94 @@ test("branchFromRefspec ignores refspecs that cannot name a pushed branch", () = assert.equal(branchFromRefspec("refs/tags/v1.0.0"), undefined); }); +test("parseGitPushArgs handles repository option forms", () => { + assert.deepEqual(parseGitPushArgs(["--repo", "origin", "feature"]), { + branch: "feature", + intent: "branch-update", + remote: "origin", + setsUpstream: false, + }); + assert.deepEqual(parseGitPushArgs(["--repo=origin", "feature"]), { + branch: "feature", + intent: "branch-update", + remote: "origin", + setsUpstream: false, + }); + assert.deepEqual( + parseGitPushArgs(["--set-upstream", "--repo=origin", "HEAD:release"]), + { + branch: "release", + intent: "branch-update", + remote: "origin", + setsUpstream: true, + }, + ); +}); + +test("parseGitPushArgs respects option values and the option terminator", () => { + assert.deepEqual( + parseGitPushArgs([ + "--receive-pack", + "git-receive-pack", + "--push-option", + "ci.skip", + "origin", + "feature", + ]), + { + branch: "feature", + intent: "branch-update", + remote: "origin", + setsUpstream: false, + }, + ); + assert.deepEqual(parseGitPushArgs(["--", "origin", "feature"]), { + branch: "feature", + intent: "branch-update", + remote: "origin", + setsUpstream: false, + }); + assert.deepEqual(parseGitPushArgs(["--repo=origin", "--", "feature"]), { + branch: "feature", + intent: "branch-update", + remote: "origin", + setsUpstream: false, + }); +}); + +test("parseGitPushArgs marks non-branch push intents", () => { + assert.deepEqual(parseGitPushArgs(["--delete", "origin", "feature"]), { + branch: "feature", + intent: "non-branch", + remote: "origin", + setsUpstream: false, + }); + assert.deepEqual(parseGitPushArgs(["origin", ":feature"]), { + branch: undefined, + intent: "non-branch", + remote: "origin", + setsUpstream: false, + }); + assert.deepEqual(parseGitPushArgs(["origin", "refs/tags/v1.0.0"]), { + branch: undefined, + intent: "non-branch", + remote: "origin", + setsUpstream: false, + }); + assert.deepEqual(parseGitPushArgs(["--tags", "origin"]), { + branch: undefined, + intent: "non-branch", + remote: "origin", + setsUpstream: false, + }); + assert.deepEqual(parseGitPushArgs(["--mirror", "origin"]), { + branch: undefined, + intent: "non-branch", + remote: "origin", + setsUpstream: false, + }); +}); + test("resolveGitPushSuccessSummary parses repository options from push args", async () => { await withGitRemote( "git@github.com:rootstrap/ai-pushgate.git", @@ -83,6 +181,7 @@ test("resolveGitPushSuccessSummary parses repository options from push args", as { env }, ), { + kind: "branch-update", pullRequestUrl: "https://github.com/rootstrap/ai-pushgate/pull/new/feature", upstream: undefined, @@ -95,6 +194,7 @@ test("resolveGitPushSuccessSummary parses repository options from push args", as { env }, ), { + kind: "branch-update", pullRequestUrl: "https://github.com/rootstrap/ai-pushgate/pull/new/release", upstream: undefined, @@ -107,6 +207,7 @@ test("resolveGitPushSuccessSummary parses repository options from push args", as { env }, ), { + kind: "branch-update", pullRequestUrl: "https://github.com/rootstrap/ai-pushgate/pull/new/feature", upstream: "origin/feature", @@ -116,13 +217,49 @@ test("resolveGitPushSuccessSummary parses repository options from push args", as ); }); +test("resolveGitPushSuccessSummary skips branch copy for non-branch pushes", async () => { + await withGitRemote( + "git@github.com:rootstrap/ai-pushgate.git", + async (env) => { + assert.deepEqual( + await resolveGitPushSuccessSummary(["--delete", "origin", "feature"], { + env, + }), + { kind: "non-branch" }, + ); + assert.deepEqual( + await resolveGitPushSuccessSummary(["origin", ":feature"], { env }), + { kind: "non-branch" }, + ); + assert.deepEqual( + await resolveGitPushSuccessSummary(["origin", "refs/tags/v1.0.0"], { + env, + }), + { kind: "non-branch" }, + ); + assert.deepEqual( + await resolveGitPushSuccessSummary(["--tags", "origin"], { env }), + { kind: "non-branch" }, + ); + assert.deepEqual( + await resolveGitPushSuccessSummary(["--mirror", "origin"], { env }), + { kind: "non-branch" }, + ); + }, + ); +}); + test("resolveGitPushSuccessSummary ignores remotes that cannot become GitHub pull request URLs", async () => { await withGitRemote( "https://gitlab.com/rootstrap/ai-pushgate.git", async (env) => { assert.deepEqual( await resolveGitPushSuccessSummary(["origin", "feature"], { env }), - { pullRequestUrl: undefined, upstream: undefined }, + { + kind: "branch-update", + pullRequestUrl: undefined, + upstream: undefined, + }, ); }, ); @@ -130,7 +267,11 @@ test("resolveGitPushSuccessSummary ignores remotes that cannot become GitHub pul await withGitRemote("not a url", async (env) => { assert.deepEqual( await resolveGitPushSuccessSummary(["origin", "feature"], { env }), - { pullRequestUrl: undefined, upstream: undefined }, + { + kind: "branch-update", + pullRequestUrl: undefined, + upstream: undefined, + }, ); }); }); @@ -146,7 +287,11 @@ test("resolveGitPushSuccessSummary treats git command failures as best effort mi assert.deepEqual( await resolveGitPushSuccessSummary(["origin", "feature"], { env }), - { pullRequestUrl: undefined, upstream: undefined }, + { + kind: "branch-update", + pullRequestUrl: undefined, + upstream: undefined, + }, ); });