| author | |
| committer | |
| log | 731b1a67acb454da7a52491ab08db39c57036699 |
| tree | f5c02d246a78e3424a98808ba7c96ebdb6a1e35d |
| parent | cdfecb36a623816f8d032408247e29b007b043f1 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: codex <codex@users.noreply.github.com>3 files changed, 166 insertions(+), 15 deletions(-)
apps/server/src/vcs/VcsProcess.test.ts+71-2| ... | @@ -6,7 +6,11 @@ import * as Fiber from "effect/Fiber"; | ... | @@ -6,7 +6,11 @@ import * as Fiber from "effect/Fiber"; |
| 6 | import * as Layer from "effect/Layer"; | 6 | import * as Layer from "effect/Layer"; |
| 7 | import { TestClock } from "effect/testing"; | 7 | import { TestClock } from "effect/testing"; |
| 8 | 8 | ||
| 9 | import { VcsProcessExitError, VcsProcessTimeoutError } from "@t3tools/contracts"; | 9 | import { |
| 10 | VcsProcessExitError, | ||
| 11 | VcsProcessSpawnError, | ||
| 12 | VcsProcessTimeoutError, | ||
| 13 | } from "@t3tools/contracts"; | ||
| 10 | import * as VcsProcess from "./VcsProcess.ts"; | 14 | import * as VcsProcess from "./VcsProcess.ts"; |
| 11 | 15 | ||
| 12 | const run = (input: VcsProcess.VcsProcessInput) => | 16 | const run = (input: VcsProcess.VcsProcessInput) => |
| ... | @@ -61,14 +65,79 @@ describe("VcsProcess.run", () => { | ... | @@ -61,14 +65,79 @@ describe("VcsProcess.run", () => { |
| 61 | 65 | ||
| 62 | it.effect("fails with VcsProcessExitError for non-zero exits by default", () => | 66 | it.effect("fails with VcsProcessExitError for non-zero exits by default", () => |
| 63 | Effect.gen(function* () { | 67 | Effect.gen(function* () { |
| 68 | const secretArgument = "--token=super-secret-token"; | ||
| 69 | const secretStderr = "remote rejected super-secret-token"; | ||
| 64 | const error = yield* run({ | 70 | const error = yield* run({ |
| 65 | operation: "test.exit", | 71 | operation: "test.exit", |
| 66 | command: "node", | 72 | command: "node", |
| 67 | args: ["-e", "process.stderr.write('boom'); process.exit(2)"], | 73 | args: [ |
| 74 | "-e", | ||
| 75 | "process.stderr.write(process.argv[1]); process.exit(2)", | ||
| 76 | secretStderr, | ||
| 77 | secretArgument, | ||
| 78 | ], | ||
| 79 | cwd: process.cwd(), | ||
| 80 | }).pipe(Effect.flip); | ||
| 81 | |||
| 82 | expect(error).toBeInstanceOf(VcsProcessExitError); | ||
| 83 | expect(error).toMatchObject({ | ||
| 84 | operation: "test.exit", | ||
| 85 | command: "node", | ||
| 86 | argumentCount: 4, | ||
| 87 | exitCode: 2, | ||
| 88 | detail: "Process exited with a non-zero status.", | ||
| 89 | failureKind: "command-failed", | ||
| 90 | stderrLength: secretStderr.length, | ||
| 91 | stderrTruncated: false, | ||
| 92 | }); | ||
| 93 | expect(error.message).not.toContain(secretArgument); | ||
| 94 | expect(error.message).not.toContain(secretStderr); | ||
| 95 | }).pipe(provideLive), | ||
| 96 | ); | ||
| 97 | |||
| 98 | it.effect("classifies authentication failures without retaining stderr", () => | ||
| 99 | Effect.gen(function* () { | ||
| 100 | const secretStderr = "authentication failed for token super-secret-token"; | ||
| 101 | const error = yield* run({ | ||
| 102 | operation: "test.authentication", | ||
| 103 | command: "node", | ||
| 104 | args: ["-e", "process.stderr.write(process.argv[1]); process.exit(1)", secretStderr], | ||
| 68 | cwd: process.cwd(), | 105 | cwd: process.cwd(), |
| 69 | }).pipe(Effect.flip); | 106 | }).pipe(Effect.flip); |
| 70 | 107 | ||
| 71 | expect(error).toBeInstanceOf(VcsProcessExitError); | 108 | expect(error).toBeInstanceOf(VcsProcessExitError); |
| 109 | expect(error).toMatchObject({ | ||
| 110 | operation: "test.authentication", | ||
| 111 | command: "node", | ||
| 112 | exitCode: 1, | ||
| 113 | detail: "Authentication failed.", | ||
| 114 | failureKind: "authentication", | ||
| 115 | stderrLength: secretStderr.length, | ||
| 116 | stderrTruncated: false, | ||
| 117 | }); | ||
| 118 | expect(error.message).not.toContain(secretStderr); | ||
| 119 | expect(error.message).not.toContain("super-secret-token"); | ||
| 120 | }).pipe(provideLive), | ||
| 121 | ); | ||
| 122 | |||
| 123 | it.effect("retains spawn causes without exposing process arguments in the error message", () => | ||
| 124 | Effect.gen(function* () { | ||
| 125 | const secretArgument = "--token=super-secret-token"; | ||
| 126 | const error = yield* run({ | ||
| 127 | operation: "test.spawn", | ||
| 128 | command: "definitely-not-a-t3code-executable", | ||
| 129 | args: [secretArgument], | ||
| 130 | cwd: process.cwd(), | ||
| 131 | }).pipe(Effect.flip); | ||
| 132 | |||
| 133 | expect(error).toBeInstanceOf(VcsProcessSpawnError); | ||
| 134 | expect(error).toMatchObject({ | ||
| 135 | operation: "test.spawn", | ||
| 136 | command: "definitely-not-a-t3code-executable", | ||
| 137 | argumentCount: 1, | ||
| 138 | }); | ||
| 139 | expect(error).toHaveProperty("cause"); | ||
| 140 | expect(error.message).not.toContain(secretArgument); | ||
| 72 | }).pipe(provideLive), | 141 | }).pipe(provideLive), |
| 73 | ); | 142 | ); |
| 74 | 143 |
apps/server/src/vcs/VcsProcess.ts+47-12| ... | @@ -8,6 +8,7 @@ import { | ... | @@ -8,6 +8,7 @@ import { |
| 8 | VcsOutputDecodeError, | 8 | VcsOutputDecodeError, |
| 9 | type VcsError, | 9 | type VcsError, |
| 10 | VcsProcessExitError, | 10 | VcsProcessExitError, |
| 11 | type VcsProcessExitFailureKind, | ||
| 11 | VcsProcessSpawnError, | 12 | VcsProcessSpawnError, |
| 12 | VcsProcessTimeoutError, | 13 | VcsProcessTimeoutError, |
| 13 | } from "@t3tools/contracts"; | 14 | } from "@t3tools/contracts"; |
| ... | @@ -46,19 +47,51 @@ const DEFAULT_TIMEOUT_MS = 30_000; | ... | @@ -46,19 +47,51 @@ const DEFAULT_TIMEOUT_MS = 30_000; |
| 46 | const DEFAULT_MAX_OUTPUT_BYTES = 1_000_000; | 47 | const DEFAULT_MAX_OUTPUT_BYTES = 1_000_000; |
| 47 | const OUTPUT_TRUNCATED_MARKER = "\n\n[truncated]"; | 48 | const OUTPUT_TRUNCATED_MARKER = "\n\n[truncated]"; |
| 48 | 49 | ||
| 49 | function commandLabel(command: string, args: ReadonlyArray<string>): string { | 50 | const classifyNonZeroExit = (command: string, stderr: string): VcsProcessExitFailureKind => { |
| 50 | return [command, ...args].join(" "); | 51 | const normalized = stderr.toLowerCase(); |
| 51 | } | 52 | |
| 53 | if ( | ||
| 54 | normalized.includes("authentication failed") || | ||
| 55 | normalized.includes("not logged in") || | ||
| 56 | normalized.includes("gh auth login") || | ||
| 57 | normalized.includes("glab auth login") || | ||
| 58 | normalized.includes("az devops login") || | ||
| 59 | normalized.includes("please run az login") || | ||
| 60 | normalized.includes("no oauth token") || | ||
| 61 | normalized.includes("unauthorized") | ||
| 62 | ) { | ||
| 63 | return "authentication"; | ||
| 64 | } | ||
| 65 | |||
| 66 | if ( | ||
| 67 | (command === "gh" && | ||
| 68 | (normalized.includes("could not resolve to a pullrequest") || | ||
| 69 | normalized.includes("repository.pullrequest") || | ||
| 70 | normalized.includes("no pull requests found for branch") || | ||
| 71 | normalized.includes("pull request not found"))) || | ||
| 72 | (command === "glab" && | ||
| 73 | (normalized.includes("merge request not found") || | ||
| 74 | normalized.includes("not found") || | ||
| 75 | normalized.includes("404"))) || | ||
| 76 | (command === "az" && | ||
| 77 | normalized.includes("pull request") && | ||
| 78 | (normalized.includes("not found") || normalized.includes("does not exist"))) | ||
| 79 | ) { | ||
| 80 | return "not-found"; | ||
| 81 | } | ||
| 82 | |||
| 83 | return "command-failed"; | ||
| 84 | }; | ||
| 52 | 85 | ||
| 53 | export const make = Effect.gen(function* () { | 86 | export const make = Effect.gen(function* () { |
| 54 | const processRunner = yield* ProcessRunner.ProcessRunner; | 87 | const processRunner = yield* ProcessRunner.ProcessRunner; |
| 55 | 88 | ||
| 56 | const run = Effect.fn("VcsProcess.run")(function* (input: VcsProcessInput) { | 89 | const run = Effect.fn("VcsProcess.run")(function* (input: VcsProcessInput) { |
| 57 | const label = commandLabel(input.command, input.args); | ||
| 58 | const baseError = { | 90 | const baseError = { |
| 59 | operation: input.operation, | 91 | operation: input.operation, |
| 60 | command: label, | 92 | command: input.command, |
| 61 | cwd: input.cwd, | 93 | cwd: input.cwd, |
| 94 | argumentCount: input.args.length, | ||
| 62 | }; | 95 | }; |
| 63 | 96 | ||
| 64 | const result = yield* processRunner | 97 | const result = yield* processRunner |
| ... | @@ -97,13 +130,15 @@ export const make = Effect.gen(function* () { | ... | @@ -97,13 +130,15 @@ export const make = Effect.gen(function* () { |
| 97 | } | 130 | } |
| 98 | 131 | ||
| 99 | if (!input.allowNonZeroExit && result.code !== 0) { | 132 | if (!input.allowNonZeroExit && result.code !== 0) { |
| 100 | return yield* new VcsProcessExitError({ | 133 | return yield* VcsProcessExitError.fromProcessExit( |
| 101 | operation: input.operation, | 134 | baseError, |
| 102 | command: label, | 135 | { |
| 103 | cwd: input.cwd, | 136 | exitCode: result.code, |
| 104 | exitCode: result.code, | 137 | stderr: result.stderr, |
| 105 | detail: result.stderr.trim() || `${label} exited with code ${result.code}.`, | 138 | stderrTruncated: result.stderrTruncated, |
| 106 | }); | 139 | }, |
| 140 | classifyNonZeroExit(input.command, result.stderr), | ||
| 141 | ); | ||
| 107 | } | 142 | } |
| 108 | 143 | ||
| 109 | return { | 144 | return { |
packages/contracts/src/vcs.ts+48-1| ... | @@ -1,5 +1,5 @@ | ... | @@ -1,5 +1,5 @@ |
| 1 | import * as Schema from "effect/Schema"; | 1 | import * as Schema from "effect/Schema"; |
| 2 | import { TrimmedNonEmptyString } from "./baseSchemas.ts"; | 2 | import { NonNegativeInt, TrimmedNonEmptyString } from "./baseSchemas.ts"; |
| 3 | 3 | ||
| 4 | export const VcsDriverKind = Schema.Literals(["git", "jj", "unknown"]); | 4 | export const VcsDriverKind = Schema.Literals(["git", "jj", "unknown"]); |
| 5 | export type VcsDriverKind = typeof VcsDriverKind.Type; | 5 | export type VcsDriverKind = typeof VcsDriverKind.Type; |
| ... | @@ -62,6 +62,7 @@ export interface VcsProcessErrorContext { | ... | @@ -62,6 +62,7 @@ export interface VcsProcessErrorContext { |
| 62 | readonly operation: string; | 62 | readonly operation: string; |
| 63 | readonly command: string; | 63 | readonly command: string; |
| 64 | readonly cwd: string; | 64 | readonly cwd: string; |
| 65 | readonly argumentCount?: number; | ||
| 65 | } | 66 | } |
| 66 | 67 | ||
| 67 | export interface VcsProcessSpawnFailure { | 68 | export interface VcsProcessSpawnFailure { |
| ... | @@ -86,12 +87,26 @@ export interface VcsProcessTimeoutFailure { | ... | @@ -86,12 +87,26 @@ export interface VcsProcessTimeoutFailure { |
| 86 | readonly timeoutMs: number; | 87 | readonly timeoutMs: number; |
| 87 | } | 88 | } |
| 88 | 89 | ||
| 90 | export const VcsProcessExitFailureKind = Schema.Literals([ | ||
| 91 | "authentication", | ||
| 92 | "not-found", | ||
| 93 | "command-failed", | ||
| 94 | ]); | ||
| 95 | export type VcsProcessExitFailureKind = typeof VcsProcessExitFailureKind.Type; | ||
| 96 | |||
| 97 | export interface VcsProcessExitFailure { | ||
| 98 | readonly exitCode: number; | ||
| 99 | readonly stderr: string; | ||
| 100 | readonly stderrTruncated: boolean; | ||
| 101 | } | ||
| 102 | |||
| 89 | export class VcsProcessSpawnError extends Schema.TaggedErrorClass<VcsProcessSpawnError>()( | 103 | export class VcsProcessSpawnError extends Schema.TaggedErrorClass<VcsProcessSpawnError>()( |
| 90 | "VcsProcessSpawnError", | 104 | "VcsProcessSpawnError", |
| 91 | { | 105 | { |
| 92 | operation: Schema.String, | 106 | operation: Schema.String, |
| 93 | command: Schema.String, | 107 | command: Schema.String, |
| 94 | cwd: Schema.String, | 108 | cwd: Schema.String, |
| 109 | argumentCount: Schema.optional(NonNegativeInt), | ||
| 95 | cause: Schema.Defect(), | 110 | cause: Schema.Defect(), |
| 96 | }, | 111 | }, |
| 97 | ) { | 112 | ) { |
| ... | @@ -113,13 +128,43 @@ export class VcsProcessExitError extends Schema.TaggedErrorClass<VcsProcessExitE | ... | @@ -113,13 +128,43 @@ export class VcsProcessExitError extends Schema.TaggedErrorClass<VcsProcessExitE |
| 113 | operation: Schema.String, | 128 | operation: Schema.String, |
| 114 | command: Schema.String, | 129 | command: Schema.String, |
| 115 | cwd: Schema.String, | 130 | cwd: Schema.String, |
| 131 | argumentCount: Schema.optional(NonNegativeInt), | ||
| 116 | exitCode: Schema.Number, | 132 | exitCode: Schema.Number, |
| 117 | detail: Schema.String, | 133 | detail: Schema.String, |
| 134 | failureKind: Schema.optional(VcsProcessExitFailureKind), | ||
| 135 | stderrLength: Schema.optional(NonNegativeInt), | ||
| 136 | stderrTruncated: Schema.optional(Schema.Boolean), | ||
| 118 | }, | 137 | }, |
| 119 | ) { | 138 | ) { |
| 120 | override get message(): string { | 139 | override get message(): string { |
| 121 | return `VCS process failed in ${this.operation}: ${this.command} (${this.cwd}) exited with ${this.exitCode} - ${this.detail}`; | 140 | return `VCS process failed in ${this.operation}: ${this.command} (${this.cwd}) exited with ${this.exitCode} - ${this.detail}`; |
| 122 | } | 141 | } |
| 142 | |||
| 143 | static fromProcessExit( | ||
| 144 | context: VcsProcessErrorContext, | ||
| 145 | error: VcsProcessExitFailure, | ||
| 146 | failureKind: VcsProcessExitFailureKind, | ||
| 147 | ) { | ||
| 148 | const detail = | ||
| 149 | failureKind === "authentication" | ||
| 150 | ? "Authentication failed." | ||
| 151 | : failureKind === "not-found" | ||
| 152 | ? context.command === "glab" | ||
| 153 | ? "Merge request not found." | ||
| 154 | : context.command === "gh" || context.command === "az" | ||
| 155 | ? "Pull request not found." | ||
| 156 | : "VCS resource not found." | ||
| 157 | : "Process exited with a non-zero status."; | ||
| 158 | |||
| 159 | return new VcsProcessExitError({ | ||
| 160 | ...context, | ||
| 161 | exitCode: error.exitCode, | ||
| 162 | detail, | ||
| 163 | failureKind, | ||
| 164 | stderrLength: error.stderr.length, | ||
| 165 | stderrTruncated: error.stderrTruncated, | ||
| 166 | }); | ||
| 167 | } | ||
| 123 | } | 168 | } |
| 124 | 169 | ||
| 125 | export class VcsProcessTimeoutError extends Schema.TaggedErrorClass<VcsProcessTimeoutError>()( | 170 | export class VcsProcessTimeoutError extends Schema.TaggedErrorClass<VcsProcessTimeoutError>()( |
| ... | @@ -128,6 +173,7 @@ export class VcsProcessTimeoutError extends Schema.TaggedErrorClass<VcsProcessTi | ... | @@ -128,6 +173,7 @@ export class VcsProcessTimeoutError extends Schema.TaggedErrorClass<VcsProcessTi |
| 128 | operation: Schema.String, | 173 | operation: Schema.String, |
| 129 | command: Schema.String, | 174 | command: Schema.String, |
| 130 | cwd: Schema.String, | 175 | cwd: Schema.String, |
| 176 | argumentCount: Schema.optional(NonNegativeInt), | ||
| 131 | timeoutMs: Schema.Number, | 177 | timeoutMs: Schema.Number, |
| 132 | }, | 178 | }, |
| 133 | ) { | 179 | ) { |
| ... | @@ -149,6 +195,7 @@ export class VcsOutputDecodeError extends Schema.TaggedErrorClass<VcsOutputDecod | ... | @@ -149,6 +195,7 @@ export class VcsOutputDecodeError extends Schema.TaggedErrorClass<VcsOutputDecod |
| 149 | operation: Schema.String, | 195 | operation: Schema.String, |
| 150 | command: Schema.String, | 196 | command: Schema.String, |
| 151 | cwd: Schema.String, | 197 | cwd: Schema.String, |
| 198 | argumentCount: Schema.optional(NonNegativeInt), | ||
| 152 | detail: Schema.String, | 199 | detail: Schema.String, |
| 153 | cause: Schema.optional(Schema.Defect()), | 200 | cause: Schema.optional(Schema.Defect()), |
| 154 | }, | 201 | }, |