Skip to content

[HYPERSHELL-107] feat(web-console): distributed tracing for browser + BFF - #144

Merged
jsell-rh merged 47 commits into
mainfrom
hypershell-27-web-console-otel
Aug 19, 2026
Merged

[HYPERSHELL-107] feat(web-console): distributed tracing for browser + BFF#144
jsell-rh merged 47 commits into
mainfrom
hypershell-27-web-console-otel

Conversation

@jsell-rh

Copy link
Copy Markdown
Collaborator

Summary

Adds OpenTelemetry distributed tracing to the HyperShell web console (browser + BFF), satisfying HYPERSHELL-27. Traces flow browser → BFF as a single W3C-propagated trace and export to a dev Jaeger instance in Kind.

Spans are derived from typed domain probes (Domain-Oriented Observability), not auto-instrumentation. OTel imports are confined to adapters/observability/** + composition/** per the architecture lint boundary.

What's included

  • Browser trace sink (app/adapters/observability/gateway-trace-sink.ts): turns gateway workflow/dependency probes into OTel spans, adopts the probe's W3C trace id, exposes traceParentFor(correlationId). Flushes buffered spans on page hide (visibilitychange/pagehide) since BatchSpanProcessor does not auto-flush.
  • W3C propagation: the fetch chokepoint (api.client.ts) injects traceparent/tracestate on /api/* calls via an injected provider (no OTel import in the API adapter).
  • BFF tracing (bff/src/adapters/observability/otel-tracing.ts): emits a SERVER span per proxied request named by bounded route template, validates/replaces inbound trace context, relays OTLP/HTTP upstream. Config-driven — disabled when OTEL_EXPORTER_OTLP_ENDPOINT is unset.
  • Dev Jaeger (deploy/kind/jaeger.yaml, scripts/kind/up.sh): opt-in via KIND_JAEGER=true make kind-up. Uses Jaeger v2 (jaegertracing/jaeger:2.20.0); OTLP receivers 4317 (gRPC, for the API server) and 4318 (HTTP, for browser/BFF) enabled by default.
  • Redaction: span names and attributes come from bounded allowlists; secrets/high-cardinality values (correlation ids, tokens, query strings) never reach the collector. Covered by tests on both sides.
  • Spec: specs/web-console/tracing.spec.md (WEB-TRACE-01..10) plus local-development spec updates.

Acceptance criteria

  • OTel Web SDK initialized in the browser with a configurable collector endpoint
  • BFF propagates incoming trace context headers upstream
  • Gateway CRUD workflows produce browser spans
  • End-to-end trace browser→BFF→API visible in Jaeger dev instance
  • Fleet-navigation spans — out of scope here (no fleet probes exist yet)

The API-server hop is intentionally out of scope; it is tracked under HYPERSHELL-26 (PR #141).

Testing

  • BFF: 56 tests pass; browser: 52 tests pass (Node 24, Vitest).
  • Live E2E in Kind: drove a real browser through Keycloak login and the gateway list workflow with Playwright; confirmed cross-service traces land in Jaeger v2 — gateway.workflow.list (browser INTERNAL) → gateway.dependency.list (browser CLIENT) → /api/* (BFF SERVER, child of the browser client span). Verified genuine W3C propagation.

🤖 Generated with Claude Code

user and others added 18 commits August 7, 2026 13:48
Define the concrete OpenTelemetry wiring for the web console as a
behavior contract that narrows WEB-OBS-01/02 and the domain-observability
standard:

- Browser spans derived from typed domain probes via a DomainProbeSink
- Same-origin BFF telemetry ingest (OTLP/HTTP); no cross-origin export
- Browser W3C traceparent injection on outbound /api/* calls
- BFF validates/replaces inbound trace context and propagates upstream
- BFF server span per proxied request, OTLP export
- Configurable, optional OTLP endpoint + sampling; privacy/cardinality rules
- 'trace' probe consumer in the catalog
- bounded delivery + flush semantics

Add a development Jaeger all-in-one to local-development.spec.md (gated by
KIND_TRACING) and register the new spec in the index.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a required createTraceId() capability to GatewayWorkflowRuntime and
publish the resulting W3C trace id in every gateway probe context, so a
trace sink can adopt it as the span trace id and probe consumers can join
a workflow to its trace (WEB-TRACE-03, UI-OBS-08). Declare `trace` as an
allowed consumer for every gateway probe in the catalog (WEB-TRACE-08).

The production runtime in the observability adapter generates a 16-byte
random trace id rendered as 32 lowercase hex digits. No OpenTelemetry
dependency is introduced by this change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…L-27]

Add an OpenTelemetry trace sink behind the gateway domain-probe port. The
sink projects gateway workflow and dependency probes onto spans, adopting
the W3C trace id carried on each probe context so a workflow joins the same
trace the browser propagates to the BFF and API. Spans batch and export
over same-origin OTLP/HTTP, flushing on document hide.

The API client gains an injected trace-context provider that stamps
traceparent/tracestate onto outbound requests, keeping the API adapter free
of any tracing vendor dependency. The composition root wires the tracer
provider, registers the sink, and feeds the propagation reader to the
client.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ngest

Add distributed tracing to the web-console BFF behind a hexagonal
tracing port. The proxy starts a SERVER span per upstream request,
continues a valid inbound W3C traceparent (else starts a new trace),
and forwards the span-derived traceparent/tracestate to the API
server. A same-origin /telemetry/v1/traces route relays browser OTLP
spans to the collector best-effort.

Tracing stays disabled unless OTEL_EXPORTER_OTLP_ENDPOINT is set, so a
deployment without a collector starts normally. The OTel SDK lives only
in the observability adapter and bootstrap; app.ts depends on the
plain BffTracing port.

Refs HYPERSHELL-27

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Deploy Jaeger all-in-one in the Kind environment, gated on KIND_JAEGER,
so a developer can view distributed traces from the web console. The
manifest mirrors the API server observability work (HYPERSHELL-26) and
additionally exposes the OTLP/HTTP receiver on 4318, which the browser
and BFF require because browsers cannot speak OTLP gRPC (4317).

When KIND_JAEGER=true, make kind-up applies the manifest, patches the
web-console Deployment with OTEL_EXPORTER_OTLP_ENDPOINT pointing at the
Jaeger OTLP/HTTP endpoint, and prints the Jaeger UI URL. Otherwise the
endpoint stays unset and the BFF starts with tracing disabled.

Reconcile the local-development spec to the KIND_JAEGER flag, the single
jaeger Service, and the dual OTLP receivers.

Refs HYPERSHELL-27

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reformat two files from the browser tracing wave that were committed
before prettier ran to completion, so pnpm run format:check passes.

Refs HYPERSHELL-27

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
BatchSpanProcessor exports on a timer, which a browser can discard when a
tab is closed or navigated away, losing the tail of a workflow trace. The
forceFlush handle existed but was never wired to any lifecycle event, so
the last spans of a gateway workflow were dropped on unload.

Register forceFlush on visibilitychange (to hidden) and pagehide, the last
reliable hooks before unload, and remove the listeners on shutdown. Correct
the docstring that falsely claimed the batch processor already flushed on
hide. Add jsdom tests covering flush on hide, no flush while visible, and
listener removal after shutdown.

Closes WEB-TRACE-09.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y data

The browser sink and BFF only ever attach an allowlisted, bounded set of
span attributes and bounded span names, so privacy held by construction but
had no test guarding it against regression.

Add redaction tests that seed a secret-shaped correlation identifier and a
URL query string carrying a fake token, then assert:
- the browser sink emits only allowlisted attribute keys and bounded span
  names, and the seeded secret appears in no span name or attribute value;
- the BFF records only the bounded route template, never the raw URL or
  query string, so request secrets never reach a span.

Closes WEB-TRACE-07.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Switch the local Kind Jaeger from the v1 all-in-one image to the v2
image (jaegertracing/jaeger:2.20.0), which is built on the OpenTelemetry
Collector. The v2 all-in-one profile enables the OTLP receivers on
4317/4318 by default, so the v1 COLLECTOR_OTLP_ENABLED env is no longer
needed. Verified live: browser -> BFF -> Jaeger cross-service traces
land under v2 with OTLP/HTTP ingest on 4318.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jsell-rh jsell-rh changed the title feat(web-console): distributed tracing for browser + BFF [HYPERSHELL-27] [HYPERSHELL-107] feat(web-console): distributed tracing for browser + BFF Aug 18, 2026
user and others added 4 commits August 18, 2026 14:15
Add WEB-TRACE-11 to the web-console tracing spec: the end-to-end trace
of WEB-TRACE-10 must be verified automatically, not only by manual
inspection. Add a matching "Web Console Distributed Trace Verification"
requirement to the e2e-testing spec (KIND_JAEGER=true for e2e, a
browser-driven cross-service trace assertion against Jaeger, and trace
failure diagnostics), plus the E2E_CONSOLE_URL/E2E_JAEGER_URL variables.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…E-11)

Add a browser-driven trace check to the platform e2e. The e2e-kind job now
brings the cluster up with KIND_JAEGER=true, installs Node + Chromium, and
runs a live Playwright spec that logs into the deployed console through
Keycloak, drives a gateway list workflow, then polls Jaeger and asserts one
trace joins the browser (bounded gateway.* span names) and the BFF server
span under a single trace id. A missing, browser-only, or BFF-only trace
fails the job.

- playwright.live.config.ts + e2e-live/tracing.live.spec.ts: live-cluster
  config and the cross-service trace assertion.
- package.json: test:e2e:live script; tsconfig.test.json: type-check the
  e2e-live sources and both playwright configs.
- Makefile: e2e-tracing target for the same check locally.
- e2e.yml: enable Jaeger, add Node/pnpm/Chromium setup, run the trace
  check, and collect Jaeger diagnostics on failure; widen the job timeout.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The gateway trace sink adopted the app-chosen trace id by manufacturing a
synthetic remote parent (a random span id that no service ever exports). Jaeger
then showed the workflow span as a child of a missing parent, so every browser
trace was decapitated and emitted spurious clock-skew warnings.

Replace the manufactured parent with a RootTraceIdGenerator on the provider:
the sink primes the next root trace id and starts the workflow span with no
parent, so it is a genuine trace root that still owns the chosen id and joins
the trace propagated to the BFF and API. Sampling moves to a
TraceIdRatioBasedSampler at the root (matching the BFF's sampler), replacing the
custom per-call decision; child dependency spans inherit it. Update WEB-TRACE-01
to require the workflow span be a true root with no synthetic remote parent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every proxied request produced a span named "/api/*" -- the Fastify catch-all
pattern -- because startProxySpan used request.routeOptions.url for both the
span name and http.route. In Jaeger this collapsed all API calls onto one
operation, erasing per-endpoint grouping and latency breakdowns.

Pass the request path (no query string) to the tracing adapter, which now
renders a bounded route template: after the /api/<group>/v<n> prefix, resource
ids collapse to {id} while collection and action segments stay literal. The
span is named "<method> <template>" (for example
"GET /api/hypershell/v1/gateways/{id}") and http.route carries the template, so
no raw id or query value is ever recorded (WEB-TRACE-07). Update WEB-TRACE-05 to
require this naming and forbid the wildcard pattern as a span name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@jsell-rh jsell-rh left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amber Analysis

The domain-probe-to-span architecture is well-factored and the current CI plus targeted browser/BFF tests are green. However, the main browser path ignores deployment sampling and several validation/reconciliation checks can report success while violating the new active tracing spec, so this is not ready to merge.

Major

  1. components/web-console/app/composition/gateway-composition.ts:13 — Browser roots default to 100% sampling, making the BFF deployment ratio ineffective. Pass an allowlisted runtime ratio/enable flag into browser composition and test ratio zero. Confidence: High (99%).
  2. components/web-console/app/adapters/observability/gateway-trace-sink.ts:285 — Asynchronous export/flush failures bypass the bounded delivery-health diagnostic. Route failures and overflow into that reporter and test rejecting/overflowing exporters. Confidence: High (97%).
  3. components/web-console/bff/src/adapters/observability/otel-tracing.ts:38 — The hand-written W3C validators accept malformed tracestate and reject valid future-version traceparent. Use a conformant propagator/parser and broaden adversarial tests. Confidence: High (99%).
  4. components/web-console/bff/src/adapters/observability/otel-tracing.ts:128 — OTLP validation checks only that resourceSpans is an array, and collector 4xx responses become 202 accepted. Validate the full bounded request and preserve rejection semantics. Confidence: High (100%).
  5. deploy/kind/jaeger.yaml:5 — Jaeger is hard-coded to hypershell-system while the script addresses ${KIND_NAMESPACE}. Render all resources and references into the selected namespace. Confidence: High (100%).
  6. scripts/kind/up.sh:261 — Turning KIND_JAEGER off does not remove Jaeger or unset the BFF endpoint on a reused cluster. Reconcile the disabled state and test the true-to-false transition. Confidence: High (99%).
  7. components/web-console/e2e-live/tracing.live.spec.ts:54 — A dependency-only browser trace satisfies the workflow-root assertion. Require distinct workflow and dependency spans. Confidence: High (100%).

Minor

  1. PR history — Nine Merge branch 'main'... commits are neither conventional nor atomic. Squash/rebase them before merge so the submitted history follows the project commit discipline. Confidence: High (100%).

Overall assessment: REQUEST_CHANGES

Findings Summary (ordered by severity, highest first):

  1. [Major] Browser sampling ignores deployment configuration - Configuration (L13)
  2. [Major] Export failures bypass bounded delivery health - Observability (L285)
  3. [Major] W3C context validation is not conformant - Input Validation (L38)
  4. [Major] Malformed nested OTLP is accepted - Input Validation (L128, L214)
  5. [Major] Jaeger ignores the selected Kind namespace - Spec Consistency (L5, L52, L73)
  6. [Major] Disabling KIND_JAEGER leaves tracing enabled - Reconciliation (L261)
  7. [Major] E2E accepts a trace without a workflow root - Verification (L54)
  8. [Minor] PR contains nine non-conventional merge commits - Commit Discipline (PR history)

Convention Checklist (omit conventions not applicable to the diff):

Convention Result
No secrets in logs or responses Pass
Input validated Fail
SecurityContext on all pod specs Pass
Configuration separate from code Fail
Desired state reconciled Fail
Observability failures bounded and surfaced Fail
Conventional commit messages Fail

// address and no cross-origin telemetry endpoint is exposed.
const browserTracesEndpoint = "/telemetry/v1/traces";

const tracing = createGatewayTracing({

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] Apply deployment sampling to the browser trace root

sampleRatio is omitted here, so createGatewayTracing defaults every browser root to 1. The BFF setting does not repair that: its ParentBasedSampler always records a sampled remote parent, so OTEL_TRACES_SAMPLE_RATIO=0 still exports all browser spans and the BFF spans descended from them. This makes the WEB-TRACE-06 deployment control ineffective on the primary request path.

Fix: expose an allowlisted tracing-enabled/sample-ratio runtime value to the browser, construct a no-op sink when disabled, and pass the same validated ratio here. Add a test showing that ratio 0 emits an unsampled context and records neither browser nor BFF spans.

Confidence: High (99%).

});

const flushBufferedSpans = (): void => {
void provider.forceFlush();

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] Surface exporter and flush failures through bounded delivery health

forceFlush() rejects when export fails or times out, but this lifecycle path discards that promise. Scheduled export failures and BatchSpanProcessor queue drops likewise stay inside the OTel diagnostic handler, so they never reach gatewayObservability.deliveryHealth() / recentDeliveryFailures(); a page-hide failure can also become an unhandled rejection. WEB-TRACE-09 and UI-OBS-05 explicitly require these failures to be best-effort and visible through the existing bounded diagnostic.

Fix: inject a bounded failure reporter into the tracing adapter/exporter, catch lifecycle flush rejections, expose queue-overflow/export failure health, and test a rejecting/overflowing exporter. If these custom hide handlers remain, disable the processor's built-in document-hide auto-flush to avoid duplicate flush paths.

Confidence: High (97%).

// A permissive W3C `tracestate`: comma-separated members, bounded length. The
// value is untrusted and only forwarded, never parsed, so a light structural
// check is enough to drop obvious garbage before propagation.
const tracestatePattern = /^[ \t]*[!-~]+=[ -~]*(,[ \t]*[!-~]+=[ -~]*){0,31}$/u;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] Use a W3C-conformant context parser

This permissive expression accepts malformed tracestate values such as Vendor=1, foo==bar, duplicate keys, and trailing value whitespace, then forwards them across the trust boundary. The adjacent traceparent expression also accepts only version 00, so a valid future-version header with extension fields is unnecessarily discarded. That conflicts with WEB-TRACE-04's requirement to forward only valid state while continuing valid W3C context.

Fix: use the OpenTelemetry W3C propagator/TraceState parser (or implement the full W3C ABNF and versioning rules), attach the parsed state to the parent context, and add malformed-state plus future-version tests.

Confidence: High (99%).

return (
typeof payload === "object" &&
payload !== null &&
Array.isArray((payload as { resourceSpans?: unknown }).resourceSpans)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] Reject malformed OTLP below the top-level array

This treats any array as valid OTLP, so payloads such as { "resourceSpans": [42] } or { "resourceSpans": ["secret"] } are forwarded. If the collector rejects one with a 4xx, line 214 maps that to unavailable, and the route still returns 202 accepted. The endpoint therefore does not satisfy WEB-TRACE-02 or the input-validation convention.

Fix: decode/validate an OTLP ExportTraceServiceRequest with bounded nested fields before forwarding, and distinguish collector 4xx rejection from 5xx/network unavailability. Add nested-malformation and collector-400 tests.

Confidence: High (100%).

Comment thread deploy/kind/jaeger.yaml Outdated
kind: Deployment
metadata:
name: jaeger
namespace: hypershell-system

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] Deploy Jaeger into the selected Kind namespace

All three resources (and the HTTPRoute parent reference) are fixed to hypershell-system, while up.sh patches and waits in ${KIND_NAMESPACE} and points the BFF at jaeger.${KIND_NAMESPACE}.svc. With a non-default namespace, Jaeger lands in the default namespace, the configured service does not exist, and kind-up times out. This directly contradicts the new target-namespace scenario.

Fix: render/include this manifest through the namespace-aware Kind deployment path so resource namespaces and parent references use the selected namespace, then cover KIND_NAMESPACE != hypershell-system in a script test.

Confidence: High (100%).

Comment thread scripts/kind/up.sh
# server observability work (HYPERSHELL-26). The web console browser and BFF
# export over OTLP/HTTP (4318) because browsers cannot speak OTLP gRPC; the API
# server uses gRPC (4317).
if [[ "${KIND_JAEGER:-}" == "true" ]]; then

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] Reconcile the tracing-disabled state

There is only an enable branch. After one KIND_JAEGER=true make kind-up, rerunning without the flag leaves the standalone Jaeger workload/route and the kubectl set env value in place; the normal manifest apply does not remove that out-of-band environment entry. The new Tracing Disabled scenario therefore fails on reused clusters.

Fix: add an idempotent disabled branch that removes these exact optional resources and unsets OTEL_EXPORTER_OTLP_ENDPOINT (then waits for the web-console rollout), with a true-to-false transition test.

Confidence: High (99%).

const service = serviceOf(trace, span);
if (
service === browserService &&
boundedSpanName.test(span.operationName)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] Require the browser workflow root in the e2e assertion

boundedSpanName matches both gateway.workflow.* and gateway.dependency.*, so a dependency span sets hasBrowserWorkflow = true. A trace whose workflow root was dropped or never exported but still contains the dependency and BFF spans passes this check—the exact decapitated trace WEB-TRACE-01/11 is meant to prevent.

Fix: use distinct workflow/dependency matchers and require at least the bounded workflow root (and the dependency span required by WEB-TRACE-11), then unit-test this predicate with a dependency-only trace.

Confidence: High (100%).

@jsell-rh jsell-rh added amber/self-review This PR was reviewed by the Amber review agent by one of the contributors to the PR. amber/changes-requested Amber requested changes on this PR labels Aug 18, 2026
user and others added 4 commits August 18, 2026 15:35
…ser spans in e2e

The live trace check accepted any bounded browser span (workflow or
dependency), so a dependency-only browser trace with no workflow root
satisfied the WEB-TRACE-11 assertion. Split the bounded name pattern into
distinct workflow and dependency templates and require all three of: a
browser workflow span that is a true root (no CHILD_OF reference), a
browser dependency child span, and a BFF server span, all under one trace
id. A decapitated or partial browser trace now fails the check.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jaeger.yaml hard-coded the namespace to hypershell-system on the
Deployment, Service, and HTTPRoute (and the HTTPRoute parentRef), while
the up.sh Jaeger block patches, waits, and sets the BFF exporter endpoint
against ${KIND_NAMESPACE}. Under any non-default namespace the two
diverged: Jaeger landed in hypershell-system but everything referencing it
pointed at the selected namespace, so trace export broke.

Make jaeger.yaml a template whose namespace and references use
${KIND_NAMESPACE}, and render it with `envsubst '${KIND_NAMESPACE}'` at
apply time so all resources and references land in the selected namespace.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When KIND_JAEGER was not set, up.sh skipped the Jaeger block entirely, so
a cluster first brought up with tracing enabled kept the Jaeger workload
and the BFF OTEL_EXPORTER_OTLP_ENDPOINT on a later run with tracing off.
The BFF then kept exporting to a collector that was gone.

Add an else branch that reconciles the disabled state: delete the rendered
Jaeger resources with --ignore-not-found and unset the BFF exporter
endpoint. Both steps are idempotent on a cluster that never had Jaeger.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tor rejection

The BFF ingest checked only that resourceSpans was an array, so a
structurally malformed body was relayed, and a collector 4xx was folded
into "unavailable" and returned to the browser as 202 accepted -- hiding a
rejected payload.

Validate the OTLP/HTTP envelope structurally and within bounds
(resourceSpans -> scopeSpans -> spans, each an object array under a fixed
cap) before relaying, and split the collector response: 4xx (other than
transient 408/429) becomes "rejected" so the browser learns its telemetry
was bad, while 408/429 and every 5xx stay "unavailable" best-effort. The
ingest route already maps "rejected" to 400 and "unavailable" to 202.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jsell-rh

Copy link
Copy Markdown
Collaborator Author

Fifth re-review: availability finding addressed

The remaining Major finding is resolved in commit 541102d.

# Sev Finding Fix
1 Major BigInt(text) parsed the whole caller-controlled decimal before the range comparison (O(n^2)); a ~1M-digit timestamp under the 1 MiB body limit blocked the event loop ~125ms even though the value was rejected withinBigIntRange now strips the sign and leading zeros, then rejects on significant-digit length (>20 digits is unconditionally out of range) before constructing a BigInt. The BigInt is built only from the bounded significant digits, so a body-sized leading-zero run never reaches it -- validation is now a linear scan. Added near-body-limit adversarial tests (all-9s and leading-zeros-then-out-of-range) asserting rejection without relay and completion under 100ms.
2 Minor Nine Merge branch main commits remain Unchanged: the project's squash-on-merge collapses the branch to a single conventional commit on main; interactive rebase is unavailable in this environment and force-pushing the shared PR branch is riskier than letting the configured merge strategy resolve it.

Verification

  • BFF: 110 tests pass (otel-tracing 55/55, up from 53); the two new body-sized cases reject in a linear scan well under the 100ms ceiling (pre-fix was ~125ms)
  • tsc (src + test) exit 0; eslint --max-warnings=0 clean; prettier clean
  • No em dash / forbidden terms; lefthook pre-commit policy checks all pass

@jsell-rh jsell-rh left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The significant-digit guard fully resolves the availability finding: near-limit decimal inputs now reject in 2.9-5.5 ms while a million leading zeros followed by an in-range value still validates. Focused tests and typechecks are clean, and I found no new code findings; this is ready provided the PR uses squash merge as required by the project convention.

Minor note

  1. PR history still contains nine Merge branch main commits, but no branch rewrite is needed if this PR is merged with GitHub squash merge. Keep squash selected so main receives one conventional commit. Confidence: High (100%).

Verification

  • BFF tracing suite: 55/55 passed.
  • BFF source and test typechecks passed.
  • The two near-limit adversarial payloads reject in 2.9-5.5 ms, down from 125-129 ms.
  • A body-sized leading-zero encoding of an in-range value still passes validation.
  • git diff --check and bash -n scripts/kind/up.sh passed.
  • The latest remote head is 541102d; completed CI checks are green, with remaining current-head checks still running at submission time.

Overall assessment: APPROVE (submitted as COMMENT because GitHub does not permit an approval review when this account owns the PR).

Findings Summary (ordered by severity, highest first):

  1. [Minor] Use squash merge so the existing branch merge commits do not enter main - Commit Discipline (PR history)

Convention Checklist (omit conventions not applicable to the diff):

Convention Result
No secrets in logs or responses Pass
Input validation is bounded Pass
Best-effort observability failures surfaced Pass
Reconciliation errors propagated Pass
Restricted SecurityContext on pod specs Pass
Configuration separate from code Pass
Image references consistent across manifests Pass
Conventional commit on main Pass (squash merge required)

@jsell-rh jsell-rh added amber/approved The Amber review agent has approved this PR. and removed amber/changes-requested Amber requested changes on this PR labels Aug 18, 2026
@jsell-rh
jsell-rh enabled auto-merge August 18, 2026 22:27
@jsell-rh
jsell-rh added this pull request to the merge queue Aug 18, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 18, 2026
@jsell-rh
jsell-rh added this pull request to the merge queue Aug 19, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 19, 2026
jsell-rh and others added 2 commits August 19, 2026 09:46
…ions

The "keeps unknown gateway status readable in every theme" e2e test scans
with axe immediately after toggling dark mode. PatternFly animates theme
colors through a CSS transition, so axe could sample a mid-transition color
and report a false color-contrast violation on the connection prereq alert's
action link, whose settled dark-mode color meets AA (~10:1 for #b9dafc on
#292929). Disable transitions and animations for this test so every scan
observes settled colors.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: openshift-online/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bffa7c91-d639-4ef8-bdd5-0f1ddcf08d01


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jsell-rh
jsell-rh enabled auto-merge August 19, 2026 14:11
The E2E Kind job's "Install Chromium" step ran `playwright install
--with-deps chromium` with no apt network timeout, so a slow or flaky Azure
Ubuntu mirror could hang the job indefinitely while fetching font packages.
Apply the same apt retry/timeout caps the web-console lint job already uses
(#145) so a stalled mirror fails fast and retries instead of wedging the run.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jsell-rh
jsell-rh added this pull request to the merge queue Aug 19, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 19, 2026
user and others added 2 commits August 19, 2026 11:32
Replace the transition-disable test workaround (f258a39) with the
source-level fix from #152 so the two PRs do not conflict and #144 can
merge first: render the install-docs link as a plain anchor instead of
AlertActionLink, and exclude the secondary menu-toggle from the axe scan
in application-shell.spec.ts. These files are now byte-identical to #152.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The merge queue resolves an ephemeral queue commit to on-pr-<merge_sha>,
which Konflux never builds, so resolve-images always falls back to the
baseline web-console image. That baseline lags main and cannot contain
not-yet-merged tracing code, so the deployed console emits no spans and
the live cross-service trace assertion (tracing.live.spec.ts) times out
at zero traces -- testing the wrong artifact.

Gate the trace verification and its Node/Chromium setup to pull_request,
push, and workflow_dispatch, where a tracing-capable image is actually
deployed. Coverage is preserved by the pull_request run (freshly built
PR image) and the push-to-main run (post-merge image).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jsell-rh
jsell-rh enabled auto-merge August 19, 2026 15:35
jsell-rh and others added 3 commits August 19, 2026 11:40
Playwright's `playwright install --with-deps chromium` runs apt-get as root
to pull Chromium's system libraries, and that apt phase intermittently
stalls indefinitely on a slow package mirror (azure.archive.ubuntu.com),
hanging the job until timeout (actions/runner-images#11347).

Drop `--with-deps` so apt is no longer on the critical path -- the
ubuntu-24.04 runner image already ships Chromium's system libraries. Cache
the browser download (keyed on the resolved Playwright version, default
branch writes only) and add a 5-minute step timeout as a backstop.

Mirrors the fix in coder/coder#28232 and coder/pixel-storybook#67.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jsell-rh
jsell-rh force-pushed the hypershell-27-web-console-otel branch from 3fafac3 to 0c80b57 Compare August 19, 2026 17:05
…into hypershell-27-web-console-otel

# Conflicts:
#	packages/gateway-management-ui/src/gateways/gateway-connection-steps.tsx
@jsell-rh
jsell-rh added this pull request to the merge queue Aug 19, 2026
@jsell-rh
jsell-rh removed this pull request from the merge queue due to a manual request Aug 19, 2026
@jsell-rh
jsell-rh merged commit a56dbc0 into main Aug 19, 2026
17 checks passed
@jsell-rh
jsell-rh deleted the hypershell-27-web-console-otel branch August 19, 2026 17:28
squizzi added a commit that referenced this pull request Aug 19, 2026
Bring in tracing (#144), CI e2e gating (#152), and platform:admin RBAC
(#143) from main. Resolve the e2e-openshell.sh conflict by keeping both
new sections in run order: platform admin RBAC as section 10 (it deletes
the gateway) and namespace garbage collection as section 11 (it validates
the namespace disappears, with a fallback delete if the gateway remains).
Repair the semantic conflict in the gRPC RBAC interceptor test, which
must now pass the new JWTRoleSyncer argument (nil, as it does not
exercise role syncing).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

amber/approved The Amber review agent has approved this PR. amber/self-review This PR was reviewed by the Amber review agent by one of the contributors to the PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant