Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
40 changes: 38 additions & 2 deletions apps/ade-cli/src/headlessLinearServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import type {
GitHubCredentialVerification,
GitHubRepoRef,
GitHubRateLimitState,
GitHubRequestBudget,
GitHubStatus,
CtoAttentionState,
} from "../../desktop/src/shared/types";
Expand Down Expand Up @@ -89,6 +90,7 @@ import {
clearGithubCredentialHealth,
githubBackgroundRequestPauseUntilMs,
githubCredentialCooldown,
githubRequestBudget,
githubCredentialNonRateLimitCooldown,
githubCredentialRateLimitCooldown,
githubCredentialInventoryKey,
Expand Down Expand Up @@ -1232,13 +1234,35 @@ export function createHeadlessGitHubService(
releaseConditionalRequest = conditional.release;
}
}
// A request that never got an answer from GitHub — a hang, a timeout, a
// DNS or TLS failure, a body that stalls mid-stream — used to throw
// straight out of here recording nothing. That is the outage shape this
// lane targets, and with no record the request budget reported no failure
// kind, so the caller's ladder could not climb past its flat unclassified
// rung. Recorded with a null rate limit so it cannot clobber the real
// quota numbers, and the kinds this produces (`network` / `unknown`)
// carry no cooldown, so it can never park a credential the user's next
// action needs.
const recordTransportFailure: (error: unknown) => never = (error) => {
recordGithubOperationFailure(
candidate,
classifyGitHubAuthFailure({
message: error instanceof Error ? error.message : String(error),
}).authFailure,
null,
);
throw error;
};

let response: Response;
try {
response = await requestGitHub(url, {
method: args.method,
headers,
body: args.body == null ? undefined : JSON.stringify(args.body),
});
} catch (error) {
recordTransportFailure(error);
} finally {
releaseConditionalRequest?.();
}
Expand All @@ -1254,9 +1278,13 @@ export function createHeadlessGitHubService(
method: args.method,
headers,
body: args.body == null ? undefined : JSON.stringify(args.body),
});
}).catch(recordTransportFailure);
}
const text = await response.text();
// The body is read after the header timer is cleared, so a socket error
// mid-body surfaces here rather than above — same shape, same record.
// (Unlike the desktop owner, this transport arms no body timeout, so a
// body that stalls without erroring simply never settles.)
const text = await response.text().catch(recordTransportFailure);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
let data: unknown = text;
try {
data = text.trim().length ? JSON.parse(text) : {};
Expand Down Expand Up @@ -1922,6 +1950,14 @@ export function createHeadlessGitHubService(
githubOperationCredentialCandidates(inventory.candidates, "read"),
);
},
/**
* Runtime-owned twin of the desktop service's budget read. The daemon owns
* GitHub access for runtime-bound windows, so implementing this only on the
* desktop side would leave the shipping build's poll governor un-gated.
*/
async getRequestBudget(): Promise<GitHubRequestBudget> {
return githubRequestBudget();
},
async getRemoteStatus() {
const origin = await readGitOriginAsync(projectRoot);
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ describe("createSyncRemoteCommandService", () => {
"agentChat.getEventHistoryPage",
"github.getStatus",
"github.getRemoteStatus",
"github.getRequestBudget",
"ai.getStatus",
"prs.list",
"prs.listOpenForRepo",
Expand Down Expand Up @@ -2088,6 +2089,7 @@ describe("createSyncRemoteCommandService", () => {
"history.listOperations",
"github.getStatus",
"github.getRemoteStatus",
"github.getRequestBudget",
"github.publishCurrentProject",
"projectConfig.get",
"projectConfig.save",
Expand Down
6 changes: 6 additions & 0 deletions apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ import type {
GitRevertArgs,
GitGetUserIdentityArgs,
GitHubRepoRef,
GitHubRequestBudget,
GitHubStatus,
GitStashPushArgs,
GitStashRefArgs,
Expand Down Expand Up @@ -5338,6 +5339,11 @@ function registerMiscRemoteCommands({ args, register }: RemoteCommandRegistratio
}));
register("github.getRemoteStatus", { viewerAllowed: true, observesAbort: true }, async (): Promise<{ repo: GitHubRepoRef | null; hasOrigin: boolean }> =>
requireService(args.githubService, "GitHub service not available.").getRemoteStatus());
// The web client's PR timers run in the browser but spend THIS machine's
// quota. Without this registration its adapter falls back to an all-null
// budget and its 5-second checks loop never sees the reserve.
register("github.getRequestBudget", { viewerAllowed: true, observesAbort: true }, async (): Promise<GitHubRequestBudget> =>
requireService(args.githubService, "GitHub service not available.").getRequestBudget());
register("github.publishCurrentProject", { viewerAllowed: true }, async (payload): Promise<PublishProjectResult> => {
const { owner, name, description, isPrivate } = parsePublishCurrentProjectArgs(payload);
return await requireService(args.githubService, "GitHub service not available.").publishCurrentProject({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,51 @@ describe("rightPaneFormatters", () => {
expect(body).toContain("CI: not run");
});

it("shows a failed checks read as a failure, not as an empty suite", () => {
// `prService.getChecks` now REJECTS when neither checks source could be
// read, and `/pr checks` catches that into `{ error }`. Rendering that as
// "No PR checks." would tell the reader the commit ran nothing — the same
// false all-clear the host-side change exists to stop.
const body = formatPrChecks({ error: "GitHub API rate limit exceeded" });

expect(body).toContain("could not be read");
expect(body).toContain("GitHub API rate limit exceeded");
expect(body).not.toContain("No PR checks.");
});

it("shows a failed comments read as a failure", () => {
const body = formatPrComments({ error: "fetch failed" });

expect(body).toContain("could not be read");
expect(body).toContain("fetch failed");
expect(body).not.toContain("No actionable PR comments.");
});

it("names the sources that failed in a partial PR review read", () => {
// `/pr review` fires three reads and catches each independently, so a
// partial outage is the common case. The sources that answered still
// render; the ones that did not must not report a count of zero.
const body = formatPrReview({
reviews: [{ state: "approved", reviewer: "octocat" }],
threads: { error: "GitHub is unavailable" },
comments: { error: "GitHub is unavailable" },
});

expect(body).toContain("1 review");
expect(body).toContain("threads unavailable");
expect(body).toContain("comments unavailable");
expect(body).toContain("GitHub is unavailable");
expect(body).not.toContain("0 threads");
expect(body).not.toContain("No PR reviews or comments.");
});

it("keeps the empty verdict when every PR review read succeeded", () => {
const body = formatPrReview({ reviews: [], threads: [], comments: [] });

expect(body).toContain("No PR reviews or comments.");
expect(body).not.toContain("could not be read");
});

it("summarizes PR review comments and threads", () => {
const body = formatPrComments({
summary: { checksStatus: "passing", actionableComments: 2 },
Expand Down
60 changes: 58 additions & 2 deletions apps/ade-cli/src/tuiClient/rightPaneFormatters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,23 @@ function unwrapPrValue(value: unknown): { root: JsonRecord; pr: JsonRecord } | n
};
}

/**
* The message from a PR read that failed, or null when the value is real data.
*
* The `/pr` dispatch catches a rejected read into `{ error }` rather than
* blowing the pane away, so every PR formatter has to tell "GitHub refused"
* apart from "there is nothing to show". Without this the two collapse: an
* `{ error }` carries no rows, so the formatters below printed "No PR checks."
* for an outage — the exact conflation `prService.getChecks` stopped making
* when it started rejecting instead of returning `[]`. A reader cannot act on
* a lie that reads as a fact, and "no checks ran" invites a merge.
*/
function prReadFailure(value: unknown): string | null {
const root = unwrapStructured(value);
if (!isRecord(root)) return null;
return asString(root.error);
}

function firstRecordArray(value: unknown, keys: string[]): JsonRecord[] {
const root = unwrapStructured(value);
if (Array.isArray(root)) return root.filter(isRecord);
Expand Down Expand Up @@ -354,6 +371,8 @@ export function formatPrMergeState(value: unknown): string {
}

export function formatPrChecks(value: unknown): string {
const failure = prReadFailure(value);
if (failure) return `PR checks · could not be read — ${failure}`;
const root = unwrapStructured(value);
const rollup = isRecord(root) ? pickString(root, ["checksStatus"]) : null;
const reason = isRecord(root) ? pickString(root, ["checksReason"]) : null;
Expand Down Expand Up @@ -399,13 +418,43 @@ export function formatPrChecks(value: unknown): string {
].join("\n");
}

/** reviews + threads + comments — the three independent reads behind `/pr review`. */
const PR_REVIEW_SOURCES = 3;

export function formatPrReview(value: unknown): string {
const root = unwrapStructured(value);
const reviews = firstRecordArray(root, ["reviews"]);
const threads = firstRecordArray(root, ["reviewThreads", "threads"]);
const comments = firstRecordArray(root, ["comments", "issueComments"]);
// Three independent reads land here, each caught into its own `{ error }`,
// so a partial outage is normal: name the sources that failed rather than
// reporting their absence as "0 reviews".
const failures: Array<[label: string, message: string]> = [];
for (const [label, part] of [
["reviews", isRecord(root) ? root.reviews : null],
["threads", isRecord(root) ? (root.reviewThreads ?? root.threads) : null],
["comments", isRecord(root) ? (root.comments ?? root.issueComments) : null],
] satisfies Array<[string, unknown]>) {
const message = prReadFailure(part);
if (message) failures.push([label, message]);
}
const rootFailure = prReadFailure(root);
if (rootFailure) return `PR review · could not be read — ${rootFailure}`;
if (failures.length === PR_REVIEW_SOURCES) {
return [
"PR review · could not be read",
...failures.map(([label, message]) => ` ✗ ${label}: ${message}`),
].join("\n");
}
// A source that failed has no count to report — printing `0 reviews` would
// state as fact the very thing the read could not establish.
const failed = new Set(failures.map(([label]) => label));
const headerCount = (label: string, noun: string, count: number): string =>
failed.has(label) ? `${noun}s unavailable` : formatCount(noun, count);
const lines = [
`PR review · ${formatCount("review", reviews.length)} · ${formatCount("thread", threads.length)} · ${formatCount("comment", comments.length)}`,
`PR review · ${headerCount("reviews", "review", reviews.length)}`
+ ` · ${headerCount("threads", "thread", threads.length)}`
+ ` · ${headerCount("comments", "comment", comments.length)}`,
];
if (reviews.length) {
lines.push("", "Reviews");
Expand All @@ -431,11 +480,18 @@ export function formatPrReview(value: unknown): string {
lines.push(`- ${commentPreview(comment)}`);
}
}
if (!reviews.length && !threads.length && !comments.length) lines.push("", "No PR reviews or comments.");
if (failures.length > 0) {
lines.push("", "Could not be read");
for (const [label, message] of failures) lines.push(` ✗ ${label}: ${message}`);
} else if (!reviews.length && !threads.length && !comments.length) {
lines.push("", "No PR reviews or comments.");
}
return lines.join("\n");
}

export function formatPrComments(value: unknown): string {
const failure = prReadFailure(value);
if (failure) return `PR comments · could not be read — ${failure}`;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const root = unwrapStructured(value);
const summary = isRecord(root) && isRecord(root.summary) ? root.summary : null;
const threads = firstRecordArray(root, ["reviewThreads", "threads"]);
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/main/services/adeActions/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,7 @@ export const ADE_ACTION_ALLOWLIST: Partial<Record<AdeActionDomain, readonly stri
"getAppUserAuthStatus",
"getRepoOrThrow",
"getRemoteStatus",
"getRequestBudget",
"getStatus",
"createRepoAutolink",
"listRepoAutolinks",
Expand Down
Loading
Loading