refactor(sse): narrow three result unions via type predicates (#8499)

* refactor(sse): narrow three result unions via type predicates

Eight diagnostics across three executors, all the same root cause already recorded in
#8483: this workspace compiles with `strictNullChecks: false`, where a boolean-literal
discriminant narrows the positive branch but not the negative one. Reading a
failure-only field after `!result.ok` therefore leaves the full union.

  auggie.ts        2  .error       on AuggieModelResolution
  muse-spark-web   4  .error       on GraphqlResult
  notion-web       2  .retryable / .errorResult on the runOnce union

#8483 retagged its union with a string discriminant. That is the better shape when the
union is module-private and small, but it does not fit here: `resolveAuggieModel` is
exported and its tests deep-equal the literal `{ ok: true, model }` object, so retagging
would churn public API and assertions to fix a checker limitation. Each union instead
gets an explicit type predicate, which narrows correctly under these compiler settings
while leaving the shape, every call site, and the tests untouched. notion-web's inline
union is named `NotionAttempt` first so it has something to `Extract` from.

Validation: full tsc error-set diff against the base config — 335 -> 327, zero new
errors (line-number-agnostic). `typecheck:core` clean; the 6 existing test files
importing a touched executor pass.

Coverage: each predicate is a one-liner whose control flow inverts on a stray `!`, and
all three failure branches already have behavioral guards —
`auggie-executor.test.ts` (400 + /Unknown Auggie model/),
`muse-spark-web-continuation.test.ts` ("Warmup failed: …"), and
`executor-notion-web.test.ts` (nested temporarily-unavailable → retried). The two
assertions added here pin the arm that the predicate unlocks on the one union that is
exported and directly reachable.

* chore(quality): rebaseline muse-spark-web.ts for #8499 own growth

The new isGraphqlFailure() type-predicate helper (TS7 strictNullChecks:false
narrowing fix) grows the frozen file 1396->1405 (+9), irreducible per the
justification recorded in config/quality/file-size-baseline.json.

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>

---------

Co-authored-by: ikelvingo <im.kelvinwong@gmail.com>
Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
This commit is contained in:
backryun
2026-07-26 15:51:45 +09:00
committed by GitHub
parent 06326a3c80
commit 1bd1af2f48
5 changed files with 74 additions and 10 deletions

View File

@@ -1,4 +1,5 @@
{
"_rebaseline_2026_07_25_8499_ts7_result_union_predicates": "PR #8499 (backryun, chore/ts7-types-executor-scattered) own growth: muse-spark-web.ts 1396->1405 (+9, irreducible). Under this workspace's `strictNullChecks: false`, the boolean-literal discriminant on `GraphqlResult` (`{ ok: true } | { ok: false; error: string }`) narrows the positive `.ok===true` branch but leaves `!result.ok` at the full union under TS7, making `.error` unreachable to the checker at the two call sites (warmup, mode-switch). Fixed by adding a single `isGraphqlFailure()` type-predicate helper (doc comment + 3-line body) reused at both call sites instead of duplicating the predicate inline — not extractable to a shared module without splitting a single-file executor's local narrowing helper out of its own file. Covered by the existing muse-spark-web executor test suite (no behavior change, pure narrowing fix).",
"_rebaseline_2026_07_22_8131_windowshide_cloudflared_spawn": "PR #8167 (Dingding-leo, fix/windows-hide-child-process, #8131) own growth: src/lib/cloudflaredTunnel.ts 934->935 (+1, irreducible call-site wiring — the single `windowsHide: true` option added to the existing cloudflared spawn() options object so no transient conhost.exe/cmd console window flashes open on Windows). Covered by the pre-merge-fix regression test tests/unit/windows-hide-child-process-spawns-8131.test.ts (added for the two additional spawn() sites the PR missed: ServiceSupervisor.ts, versionManager/processManager.ts) plus the windowsHide assertion added to tests/unit/services/installers/runNpm-shell-5379.test.ts (installers/utils.ts buildNpmExecOptions).",
"_rebaseline_2026_07_22_8006_adobe_firefly_media_provider": "PR #8006 (artickc, feat/adobe-firefly-media) own growth: adds Adobe Firefly as a media-only (image + video) provider — unofficial IMS/cookie-session bridge for firefly.adobe.com covering IMS cookie->access_token exchange, discovery-catalog fallback, credits/balance usage, and submit+poll dispatch for both image (nano-banana/gpt-image families) and video (Sora 2/Veo 3.1/Kling 3.0) generation, with 408-under-load retry handling. New leaf open-sse/services/adobeFireflyClient.ts frozen at 1958 (>>cap 800) — a single self-contained upstream client (mirrors the qoderCli.ts precedent for a new provider client that is legitimately large on day one: IMS auth, cookie/JWT normalization, payload builders for 2 media types x multiple model families, SSE-less submit/poll state machine, error sanitization); not extractable without scattering a single upstream integration across artificial module boundaries mid-PR. open-sse/config/imageRegistry.ts (existing, previously under cap) grows 800->821 (+21, the new adobe-firefly IMAGE_PROVIDERS entry + models list, additive registry data at the existing registry chokepoint). src/lib/usage/providerLimits.ts 1000->1003 (+3, adobe-firefly/firefly added to the existing apikey-usage-fetcher allowlist, irreducible call-site wiring mirroring the sibling #7994 PromptQL/HyperAgent entries in the same PR group). Covered by tests/unit/adobe-firefly.test.ts (35/35). Structural shrink tracked in #3501.",
"_rebaseline_2026_07_22_7994_hyperagent_web_provider": "PR #7994 (artickc, feat/hyperagent-web) own growth: adds HyperAgent (hyperagent.com) as a new unofficial web-cookie chat provider, reverse-engineered from live SPA captures (thread/session SSE flow, credits/usage endpoint). New leaf open-sse/executors/hyperagent.ts frozen at 937 (>cap 800) — single self-contained executor covering cookie auth, SSE parsing (text/session_start/session_end/done events), and a sticky thread/session cache for multi-turn continuity; not extractable without splitting the executor mid-request-flow (mirrors the sseParser.ts/muse-spark-web.ts precedent for new provider executors that exceed cap on day one). src/lib/usage/providerLimits.ts 1000->1003 (+3, irreducible call-site wiring adding hyperagent/ha to the existing USAGE_FETCHER_PROVIDERS-style allowlist at the chokepoint other web-cookie providers already extend). Covered by tests/unit/executor-hyperagent.test.ts (16/16). Structural shrink tracked in #3501.",
@@ -175,7 +176,7 @@
"open-sse/executors/duckduckgo-web.ts": 925,
"open-sse/executors/grok-web.ts": 1873,
"open-sse/executors/hyperagent.ts": 937,
"open-sse/executors/muse-spark-web.ts": 1396,
"open-sse/executors/muse-spark-web.ts": 1405,
"open-sse/executors/perplexity-web.ts": 1032,
"open-sse/handlers/audioSpeech.ts": 1061,
"open-sse/handlers/chatCore.ts": 5125,

View File

@@ -149,6 +149,19 @@ export async function initAuggieModels(
type AuggieModelResolution = { ok: true; model: string } | { ok: false; error: string };
/**
* This workspace compiles with `strictNullChecks: false`, where a boolean-literal
* discriminant narrows the positive branch but not the negative one — so `!r.ok` alone
* leaves `r` as the full union and reading `.error` fails. An explicit type predicate
* narrows under those settings without retagging the union (which is public API here:
* `resolveAuggieModel` is exported and its tests deep-equal the `{ ok: true, ... }` shape).
*/
function isAuggieModelFailure(
resolution: AuggieModelResolution
): resolution is Extract<AuggieModelResolution, { ok: false }> {
return !resolution.ok;
}
/**
* Validate + resolve the requested model against the registry allowlist.
* Rejects flag-smuggling (leading "-") and any id not declared in the registry.
@@ -384,7 +397,7 @@ export class AuggieExecutor extends BaseExecutor {
await initAuggieModels(signal);
// Argument-injection defense: never forward an unvalidated model into the argv.
const modelResolution = resolveAuggieModel(model);
if (!modelResolution.ok) {
if (isAuggieModelFailure(modelResolution)) {
const response = wantsStream
? buildAuggieSseError(modelResolution.error)
: errorResponse(400, modelResolution.error);

View File

@@ -906,6 +906,15 @@ function buildWsUrl(authorization: string, requestId: string): string {
type GraphqlResult = { ok: true } | { ok: false; error: string };
/**
* Narrows the failure arm. Under this workspace's `strictNullChecks: false`, a
* boolean-literal discriminant narrows the positive branch but not the negative one, so
* `!result.ok` leaves the full union and `.error` is unreachable to the checker.
*/
function isGraphqlFailure(result: GraphqlResult): result is Extract<GraphqlResult, { ok: false }> {
return !result.ok;
}
async function graphqlPost(
docId: string,
variables: Record<string, unknown>,
@@ -1315,7 +1324,7 @@ export class MuseSparkWebExecutor extends BaseExecutor {
"Warmup",
signal
);
if (!warmupResult.ok) {
if (isGraphqlFailure(warmupResult)) {
evictContinuationIfNeeded(cached, continuationCacheKey);
log?.error?.("MUSE-SPARK-WEB", `Warmup failed: ${warmupResult.error}`);
return errorResult(502, warmupResult.error, "meta_ai_warmup_failed", {}, body);
@@ -1329,7 +1338,7 @@ export class MuseSparkWebExecutor extends BaseExecutor {
"Mode switch",
signal
);
if (!modeResult.ok) {
if (isGraphqlFailure(modeResult)) {
evictContinuationIfNeeded(cached, continuationCacheKey);
log?.error?.("MUSE-SPARK-WEB", `Mode switch failed: ${modeResult.error}`);
return errorResult(502, modeResult.error, "meta_ai_mode_switch_failed", {}, body);

View File

@@ -606,13 +606,26 @@ export class NotionWebExecutor extends BaseExecutor {
const reqHeaders = buildNotionExecuteHeaders({ cookie, spaceId, userId, agent });
type NotionAttempt =
| { ok: true; finalText: string; reqBody: Record<string, unknown> }
| {
ok: false;
errorResult: ReturnType<typeof makeErrorResult>;
retryable: boolean;
reqBody: Record<string, unknown>;
};
// `strictNullChecks: false` narrows a boolean-literal discriminant on the positive
// branch only, so `!attempt.ok` leaves the full union and the failure-only fields are
// unreachable to the checker. An explicit predicate narrows under those settings.
const isFailedAttempt = (
attempt: NotionAttempt
): attempt is Extract<NotionAttempt, { ok: false }> => !attempt.ok;
const runOnce = async (opts: {
createThread: boolean;
threadId: string;
}): Promise<
| { ok: true; finalText: string; reqBody: Record<string, unknown> }
| { ok: false; errorResult: ReturnType<typeof makeErrorResult>; retryable: boolean; reqBody: Record<string, unknown> }
> => {
}): Promise<NotionAttempt> => {
const transcript = buildNotionTranscript(messages, {
notionModel: notionCodename || undefined,
spaceId,
@@ -681,13 +694,13 @@ export class NotionWebExecutor extends BaseExecutor {
let attempt = await runOnce({ createThread, threadId });
// One automatic retry for transient Notion faults — same threadId, never create again
if (!attempt.ok && attempt.retryable) {
if (isFailedAttempt(attempt) && attempt.retryable) {
const delayMs = process.env.NODE_ENV === "test" || process.env.VITEST ? 20 : 700 + Math.floor(Math.random() * 400);
await new Promise((r) => setTimeout(r, delayMs));
attempt = await runOnce({ createThread: false, threadId });
}
if (!attempt.ok) {
if (isFailedAttempt(attempt)) {
return attempt.errorResult;
}

View File

@@ -293,6 +293,34 @@ test("resolveAuggieModel resolves every pre-v0.32.0 alias to an allowlisted v0.3
}
});
// The failure arm is read through an `isAuggieModelFailure()` type predicate — this
// workspace compiles with `strictNullChecks: false`, where `!result.ok` narrows the
// success branch but not the failure one, so `.error` was unreachable to the checker.
// The predicate is a one-liner whose control flow inverts on a stray `!`, so pin both
// arms: the failure arm must carry a usable message, the success arm must not claim one.
test("resolveAuggieModel's failure arm carries a readable error message", () => {
const result = resolveAuggieModel("totally-not-a-real-model");
assert.equal(result.ok, false);
if (!result.ok) {
assert.equal(typeof result.error, "string");
assert.ok(result.error.length > 0, "the rejection must explain itself");
assert.match(result.error, /Unknown Auggie model/);
}
});
test("resolveAuggieModel's success arm resolves a model and reports no error", () => {
const result = resolveAuggieModel("haiku4.5");
assert.equal(result.ok, true);
if (result.ok) {
assert.equal(result.model, "haiku4.5");
assert.equal(
(result as unknown as { error?: unknown }).error,
undefined,
"the success arm must not carry a failure message"
);
}
});
// ─── execute(): model allowlist (argument-injection defense) ──────────────
test("execute() rejects a model not in the registry allowlist and never spawns", async () => {