ext/hyperframes
2
0
Fork 0
mirror of https://github.com/heygen-com/hyperframes.git synced 2026-09-03 04:38:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(cli): keep a set-but-empty router env var breaker-managed (review)

Browse Source 
Ownership detection classified ANY defined HF_DE_PARALLEL_ROUTER as a user
choice, but both parsers read empty/whitespace as "unset -> default ON".
Launching with `HF_DE_PARALLEL_ROUTER=` therefore routed the render (empty
parses as ON) while exempting the install from its circuit breaker: after a
verified fallback applyDeParallelRouterBreaker() no-op'd, so the install kept
retrying the failing router instead of latching off. That is the exact
first-fallback protection this PR exists to provide, lost on a documented
default path. Ownership now uses the same normalization as the parsers.

Also: only announce a trip the breaker could act on. With an explicit user
opt-in the breaker is deliberately a no-op, so "now off for this install" was
factually wrong — and reprinted on every later revert, since the user's value
keeps the router active.

Tests: set-but-empty and whitespace both latch off and persist the fired flag
(fault-injection verified — restoring the old check fails both); explicit
"true" survives a fallback.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vance Ingalls
2026-08-07 15:01:50 -07:00
co-authored by Claude Opus 5
parent c6df112ac1
commit af535080a2
2 changed files with 59 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/cli/src/commands/render.ts
+14 -3
View File
@@ -69,7 +69,6 @@ import {
writeConfigWithResult,
type HyperframesConfig,
} from "../telemetry/config.js";
import { shouldTrack } from "../telemetry/client.js";
import { renderJobObservabilityTelemetryPayload } from "../telemetry/renderObservability.js";
import { bytesToMb } from "../telemetry/system.js";
import { VERSION } from "../version.js";
@@ -1185,8 +1184,16 @@ function applyDeParallelRouterBreaker(): void {
function applyDeParallelRouterCircuitBreaker(quiet: boolean): boolean {
// Latch the user's own choice on first observation, BEFORE the breaker can
// write the var itself and make the two indistinguishable.
//
// Ownership uses the SAME normalization as the two parsers: a set-but-empty
// (or whitespace) value means "unset / default ON", so it is NOT a user
// choice and must stay breaker-managed. Treating any defined value as
// user-managed would let `HF_DE_PARALLEL_ROUTER=` route the render (empty
// parses as ON) while exempting that install from the breaker — it would
// keep retrying a failing router forever, losing exactly the first-fallback
// protection this PR exists to provide (review finding).
if (!deParallelRouterUserManagedResolved) {
deParallelRouterUserManaged = process.env.HF_DE_PARALLEL_ROUTER !== undefined;
deParallelRouterUserManaged = (process.env.HF_DE_PARALLEL_ROUTER ?? "").trim() !== "";
deParallelRouterUserManagedResolved = true;
}
if (deParallelRouterUserManaged) {
@@ -1326,7 +1333,11 @@ function maybeConsumeDeParallelRouterTrial(
applyDeParallelRouterBreaker();
}
writeConfig(config);
if (fired) reportDeParallelRouterBreakerTrip(quiet);
// Only announce a trip the breaker could actually act on. With an explicit
// user opt-in the breaker is a no-op, so "now off for this install" would
// be false — and would reprint on every subsequent revert, since the user's
// value keeps the router active (review finding).
if (fired && !deParallelRouterUserManaged) reportDeParallelRouterBreakerTrip(quiet);
}
/**