Skip to content

Commit 52f441f

Browse files
committed
fix(rules): report a lost generation request as NETWORK_ERROR, and refuse a generated status with no revisions
request_not_found on poll names the CLI's own request id, never a rule id, so RULE_NOT_FOUND's remedy (re-check the directory name) was wrong; the remedy is to resubmit. A generated status without revisions threw a raw TypeError; it is now an invalid response, not a silent zero-rule success.
1 parent f49597b commit 52f441f

2 files changed

Lines changed: 70 additions & 6 deletions

File tree

‎packages/cli/src/rules/generate.ts‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,9 @@ export type FinishedRequest = RequestStatus;
6161
* Poll a request until it stops moving.
6262
*
6363
* Throws a `CLIError` for anything that is not an answer about the request:
64-
* `request_not_found` (the id will never resolve), a rejected token, and an
65-
* unreachable service, each with the code a caller branches on.
64+
* `request_not_found` (the id will never resolve, reported as `NETWORK_ERROR`
65+
* because the remedy is to resubmit), a rejected token, and an unreachable
66+
* service, each with the code a caller branches on.
6667
*/
6768
export async function awaitRequest(
6869
context: GenerationContext,
@@ -86,13 +87,16 @@ export async function awaitRequest(
8687
);
8788
}
8889
case "error": {
90+
// `request_not_found` names the request id this CLI submitted a moment
91+
// ago, never a rule id the caller supplied, so it is not
92+
// RULE_NOT_FOUND: that code's documented remedy is "re-check the
93+
// directory name", and `rule create` has no rule id at all. It is a
94+
// poll that failed, and the remedy is to submit again.
8995
throw new CLIError(
9096
outcome.code === "request_not_found"
91-
? `Request ${requestId} was not found for this repository.`
97+
? `Request ${requestId} is no longer known to the service for this repository. Submit the request again.`
9298
: `Polling failed (${outcome.code}).`,
93-
outcome.code === "request_not_found"
94-
? "RULE_NOT_FOUND"
95-
: "NETWORK_ERROR"
99+
"NETWORK_ERROR"
96100
);
97101
}
98102
case "refused":
@@ -161,6 +165,18 @@ export async function deliverRevisions(
161165
context: GenerationContext,
162166
revisions: FinishedRequest["revisions"]
163167
): Promise<Delivered> {
168+
// The contract requires `revisions` on every status, but `getRequestStatus`
169+
// only checks that the body is an object. A `generated` status without the
170+
// list is a malformed response, reported the way `api/v2.ts` reports any
171+
// other one. Not `?? []`: that would print "0 rule(s)" and exit cleanly for
172+
// a request that did generate something, a silent success.
173+
if (!Array.isArray(revisions)) {
174+
throw new CLIError(
175+
"The service reported the request as generated but did not list the rules it produced (invalid response body).",
176+
"NETWORK_ERROR"
177+
);
178+
}
179+
164180
const fetched = await Promise.all(
165181
revisions.map(async ({ ruleId, revisionId }) => ({
166182
ruleId,

‎packages/cli/test/rule-create-entitlement.test.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,54 @@ describe("rule create/improve: a runtime rule the plan will not run", () => {
276276
expect(envelope.message).not.toContain("\u001B");
277277
});
278278

279+
it("reports a poll whose request is gone as NETWORK_ERROR, not RULE_NOT_FOUND", async () => {
280+
stubV2Server({ produced: [] });
281+
const fetchMock = globalThis.fetch as unknown as {
282+
getMockImplementation: () => (input: Request) => Promise<Response>;
283+
mockImplementation: (f: (input: Request) => Promise<Response>) => void;
284+
};
285+
const original = fetchMock.getMockImplementation();
286+
fetchMock.mockImplementation(async (input: Request) => {
287+
const url = new URL(input.url);
288+
if (url.pathname.startsWith("/cli/api/v2/request/")) {
289+
return Response.json({ error: "request_not_found" }, { status: 404 });
290+
}
291+
return original(input);
292+
});
293+
294+
await expect(create()).rejects.toThrow();
295+
const envelope = JSON.parse(String(logSpy.mock.calls.at(-1)?.[0])) as {
296+
code?: string;
297+
message?: string;
298+
};
299+
expect(envelope.code).toBe("NETWORK_ERROR");
300+
expect(envelope.message).toContain(REQUEST_ID);
301+
});
302+
303+
it("reports a generated status with no revisions list as a malformed response", async () => {
304+
stubV2Server({ produced: [] });
305+
const fetchMock = globalThis.fetch as unknown as {
306+
getMockImplementation: () => (input: Request) => Promise<Response>;
307+
mockImplementation: (f: (input: Request) => Promise<Response>) => void;
308+
};
309+
const original = fetchMock.getMockImplementation();
310+
fetchMock.mockImplementation(async (input: Request) => {
311+
const url = new URL(input.url);
312+
if (url.pathname.startsWith("/cli/api/v2/request/")) {
313+
return Response.json({ requestId: REQUEST_ID, status: "generated" });
314+
}
315+
return original(input);
316+
});
317+
318+
await expect(create()).rejects.toThrow();
319+
const envelope = JSON.parse(String(logSpy.mock.calls.at(-1)?.[0])) as {
320+
code?: string;
321+
message?: string;
322+
};
323+
expect(envelope.code).toBe("NETWORK_ERROR");
324+
expect(envelope.message).toContain("invalid response body");
325+
});
326+
279327
it("rule improve reports an unknown rule id as RULE_NOT_FOUND", async () => {
280328
vi.stubGlobal(
281329
"fetch",

0 commit comments

Comments
 (0)