Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions apps/server/src/sourceControl/GitHubCli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,29 @@ describe("GitHubCli.layer", () => {
assert.notProperty(commandFailure, "operation");
});

it("surfaces guest App-wrapper diagnostics instead of the generic command-failed line", () => {
const context = { command: "gh", cwd: "/repo" } as const;
const cause = new VcsProcessExitError({
operation: "GitHubCli.execute",
command: "gh",
cwd: context.cwd,
exitCode: 1,
failureKind: "command-failed",
detail: "Process exited with a non-zero status.",
publicDiagnostic:
"t3-github-app-token: app is not installed on pingdotgg/t3code (or repo does not exist)",
});

const error = GitHubCli.fromVcsError(context, cause);

assert.equal(error._tag, "GitHubCliCommandError");
assert.equal(
error.detail,
"t3-github-app-token: app is not installed on pingdotgg/t3code (or repo does not exist)",
);
assert.equal(error.message.includes("app is not installed"), true);
});

it.effect("parses pull request view output", () =>
Effect.gen(function* () {
mockRun.mockReturnValueOnce(
Expand Down
14 changes: 12 additions & 2 deletions apps/server/src/sourceControl/GitHubCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,13 @@ export class GitHubPullRequestNotFoundError extends Schema.TaggedErrorClass<GitH

export class GitHubCliCommandError extends Schema.TaggedErrorClass<GitHubCliCommandError>()(
"GitHubCliCommandError",
gitHubCliFailureFields,
{
...gitHubCliFailureFields,
publicDiagnostic: Schema.optional(Schema.String),
},
) {
get detail(): string {
return "GitHub CLI command failed.";
return this.publicDiagnostic ?? "GitHub CLI command failed.";
}

override get message(): string {
Expand Down Expand Up @@ -190,6 +193,13 @@ export function fromVcsError(
if (error.failureKind === "not-found") {
return new GitHubPullRequestNotFoundError({ ...context, cause: error });
}
if (error.publicDiagnostic !== undefined) {
return new GitHubCliCommandError({
...context,
cause: error,
publicDiagnostic: error.publicDiagnostic,
});
}
}

return new GitHubCliCommandError({ ...context, cause: error });
Expand Down
44 changes: 44 additions & 0 deletions apps/server/src/vcs/VcsProcess.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,28 @@ describe("VcsProcess.run", () => {
}).pipe(provideLive),
);

it.effect("keeps guest App-wrapper diagnostics off the process error message", () =>
Effect.gen(function* () {
const wrapperLine =
"t3-github-app-token: app is not installed on pingdotgg/t3code (or repo does not exist)";
const error = yield* run({
operation: "test.wrapper-diagnostic",
command: "node",
args: ["-e", "process.stderr.write(process.argv[1]); process.exit(1)", wrapperLine],
cwd: process.cwd(),
}).pipe(Effect.flip);

expect(error).toBeInstanceOf(VcsProcessExitError);
expect(error).toMatchObject({
failureKind: "command-failed",
publicDiagnostic: wrapperLine,
});
// Operators see this on GitHubCli.detail; the VCS message stays canned.
expect(error.message).not.toContain("pingdotgg");
expect(error.message).toContain("Process exited with a non-zero status.");
}).pipe(provideLive),
);

it.effect("classifies authentication failures without retaining stderr", () =>
Effect.gen(function* () {
const secretStderr = "authentication failed for token super-secret-token";
Expand Down Expand Up @@ -297,3 +319,25 @@ describe("VcsProcess.run", () => {
}).pipe(provideLive),
);
});

describe("publicDiagnosticFromStderr", () => {
it("keeps only guest wrapper lines, and drops GraphQL bodies", () => {
expect(
VcsProcess.publicDiagnosticFromStderr(
"GraphQL: Could not resolve to a PullRequest with the number of 6613. (repository.pullRequest)",
),
).toBeUndefined();
expect(
VcsProcess.publicDiagnosticFromStderr(
"t3-github-app-token: app is not installed on pingdotgg/t3code (or repo does not exist)",
),
).toBe(
"t3-github-app-token: app is not installed on pingdotgg/t3code (or repo does not exist)",
);
expect(
VcsProcess.publicDiagnosticFromStderr(
"gh-app-wrapper: auth failed (exit 4); reminting and retrying once after 3s",
),
).toMatch(/^gh-app-wrapper:/);
});
});
16 changes: 16 additions & 0 deletions apps/server/src/vcs/VcsProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,21 @@ const classifyNonZeroExit = (command: string, stderr: string): VcsProcessExitFai
return "command-failed";
};

/** Guest App-wrapper lines are written for operators; they do not carry tokens. */
const WRAPPER_DIAGNOSTIC = /^(?:t3-github-app-token|gh-app-wrapper):.+/i;
const PUBLIC_DIAGNOSTIC_MAX = 240;

export function publicDiagnosticFromStderr(stderr: string): string | undefined {
for (const line of stderr.split(/\r?\n/)) {
const trimmed = line.trim();
if (!WRAPPER_DIAGNOSTIC.test(trimmed)) continue;
return trimmed.length > PUBLIC_DIAGNOSTIC_MAX
? `${trimmed.slice(0, PUBLIC_DIAGNOSTIC_MAX - 1)}…`
: trimmed;
}
return undefined;
}

export const make = Effect.gen(function* () {
const processRunner = yield* ProcessRunner.ProcessRunner;
const sourceControlCliSemaphore = yield* Semaphore.make(SOURCE_CONTROL_CLI_CONCURRENCY);
Expand Down Expand Up @@ -174,6 +189,7 @@ export const make = Effect.gen(function* () {
stderrTruncated: result.stderrTruncated,
},
classifyNonZeroExit(input.command, result.stderr),
publicDiagnosticFromStderr(result.stderr),
);
}

Expand Down
7 changes: 7 additions & 0 deletions packages/contracts/src/vcs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ export class VcsProcessExitError extends Schema.TaggedErrorClass<VcsProcessExitE
failureKind: Schema.optional(VcsProcessExitFailureKind),
stderrLength: Schema.optional(NonNegativeInt),
stderrTruncated: Schema.optional(Schema.Boolean),
/**
* A guest-wrapper line that is safe to show. Raw provider stderr stays off
* `message` so tokens in GraphQL/REST failures cannot leak into logs.
*/
publicDiagnostic: Schema.optional(Schema.String),
},
) {
override get message(): string {
Expand All @@ -138,6 +143,7 @@ export class VcsProcessExitError extends Schema.TaggedErrorClass<VcsProcessExitE
context: VcsProcessErrorContext,
error: VcsProcessExitFailure,
failureKind: VcsProcessExitFailureKind,
publicDiagnostic?: string,
) {
const detail =
failureKind === "authentication"
Expand All @@ -159,6 +165,7 @@ export class VcsProcessExitError extends Schema.TaggedErrorClass<VcsProcessExitE
failureKind,
stderrLength: error.stderr.length,
stderrTruncated: error.stderrTruncated,
...(publicDiagnostic !== undefined ? { publicDiagnostic } : {}),
});
}
}
Expand Down
Loading