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
35 changes: 32 additions & 3 deletions apps/server/src/provider/providerMaintenanceRunner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -665,14 +665,43 @@ describe("providerMaintenanceRunner", () => {
const updateState = result.providers[0]?.updateState;

assert.strictEqual(updateState?.status, "failed");
assert.strictEqual(updateState?.message, "Update command exited with code 1.");
assert.include(updateState?.output ?? "", "permission denied");
assert.strictEqual(
updateState?.message,
"Update command exited with code 1: Error: Could not detect the Codex installation method.",
);
assert.include(updateState?.output ?? "", "checking install");
}).pipe(
Effect.provide(
Layer.mergeAll(
layerNonWindowsPlatform,
layerLatestVersionHttpClient("0.0.0"),
layerMockSpawner(() => ({
stdout: "checking install\n",
stderr: "Error: Could not detect the Codex installation method.\n\n",
code: 1,
})),
),
),
),
);

it.effect("keeps the exit code message when a failed command prints nothing", () =>
Effect.gen(function* () {
const { registry } = yield* makeRegistry();
const updater = yield* makeTestRunner(registry);

const result = yield* updater.updateProvider(CODEX_DRIVER);

assert.strictEqual(
result.providers[0]?.updateState?.message,
"Update command exited with code 1.",
);
}).pipe(
Effect.provide(
Layer.mergeAll(
layerNonWindowsPlatform,
layerLatestVersionHttpClient("0.0.0"),
layerMockSpawner(() => ({ stderr: "permission denied", code: 1 })),
layerMockSpawner(() => ({ code: 1 })),
),
),
),
Expand Down
31 changes: 27 additions & 4 deletions apps/server/src/provider/providerMaintenanceRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,30 @@ function commandOutput(result: ProviderMaintenanceCommandResult): string | null
return truncateText(output, UPDATE_OUTPUT_MAX_BYTES);
}

// Updaters print their reason last, usually on stderr.
function lastOutputLine(result: ProviderMaintenanceCommandResult): string | null {
for (const output of [result.stderr, result.stdout]) {
const line = output
.split(/\r\n|\r|\n/)
.map(toProgressLine)
.findLast(Boolean);
if (line) {
return line;
}
}
return null;
}

function failureMessage(result: ProviderMaintenanceCommandResult): string {
if (result.timedOut) {
return "Update timed out.";
}
if (result.exitCode !== null && result.exitCode !== 0) {
return `Update command exited with code ${result.exitCode}.`;
}
return "Update command failed.";
const summary =
result.exitCode !== null && result.exitCode !== 0
? `Update command exited with code ${result.exitCode}`
: "Update command failed";
const reason = lastOutputLine(result);
return reason ? `${summary}: ${reason}` : `${summary}.`;
}

function isOutdatedProvider(provider: ServerProvider | undefined): boolean {
Expand Down Expand Up @@ -474,6 +490,13 @@ export const make = Effect.fn("ProviderMaintenanceRunner.make")(function* () {
const result = yield* runMaintenanceCommand(command, setRunningMessage);
const finishedAt = yield* nowIso;
if (result.timedOut || result.exitCode !== 0) {
yield* Effect.logWarning("Provider update command failed", {
provider,
command: describeCommand(command),
exitCode: result.exitCode,
timedOut: result.timedOut,
output: commandOutput(result),
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return yield* finish(
makeUpdateState({
status: "failed",
Expand Down
Loading