feat(e2e): performance harness with incremental checkpoints

[squizzi]

What

Implements the HYPERSHELL-18 performance-test spec: an infra-agnostic e2e performance harness with batched, checkpointed gateway scale-up.

Highlights

• Adds E2E_MODE=short|long step-depth tagging to the e2e suite (tests/e2e/lib.sh, tests/e2e/e2e-openshell.sh); long remains the default so existing CI is unchanged.
• New tests/e2e/e2e-performance.sh harness: batched gateway scale-up with bounded concurrency, per-batch short-mode checkpoints, a final long-mode functional gate, optional SLO gating, and EXIT-trap teardown.
• New tests/e2e/perf/lib.sh utilities (timing, latency percentiles, bounded concurrency, schema_version=1 JSON results I/O) plus tests/e2e/perf/lib_test.sh unit tests (no cluster required).
• New scripts/perf-report.sh and make e2e-performance / make e2e-performance-report targets to run the harness and tabulate local run history.
• Spec correction: the report depends only on bash, not python3/jq as previously stated.

Scope

Local/manual performance testing only — not wired into PR CI (too slow/heavy for the 20-minute gate). The OpenShift driver itself remains out of scope (tracked under HYPERSHELL-44).

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 00b1921b-631d-4a2a-941c-d30d02e4bf65

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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

[jsell-rh]

Amber review

Status: In progress

Amber started a review of commit 65f52c7 at 2026-08-27T18:46:49Z.

[jsell-rh]

Amber review

Status: Complete

Verdict

COMMENT (approve-with-nits). This is a well-structured, well-tested local/manual performance harness plus a partial OpenShift e2e driver and an E2E_MODE=short|long split; there is no production Go code, long remains the default so existing CI is untouched, and both cluster-free unit suites (tests/e2e/perf/lib_test.sh, tests/e2e/openshift_driver_test.sh) pass locally. My findings are all Minor (worker credential handling, a small JSON-schema default inconsistency, and nondeterministic seed selection); none block merge, but a couple of cross-PR coordination points below need a maintainer decision.

I reviewed against CLAUDE.md, security.spec.md, control-plane/conventions.spec.md, and the review checklists. I paid particular attention to Test Diff Scrutiny: the pre-existing assertions in tests/e2e/e2e-openshell.sh are gated behind if e2e_step long, and because E2E_MODE defaults to long, every previously-unconditional assertion still runs in the default/CI path — no guarantee is silently removed. The CA-extraction assertion that changed for the openshift branch is a new-driver code path, not a weakened contract for the existing kind path.

Findings

1. [Minor] Credentials spliced into bash -c worker command strings (argv exposure).
   In tests/e2e/e2e-performance.sh, both the scale-up workers (export E2E_OIDC_PASSWORD="'"${E2E_OIDC_PASSWORD}"'" and friends) and the teardown workers build a bash -c '...' string with the secret values interpolated as literals. Those values then appear in the child process argv (visible via ps//proc/<pid>/cmdline) and the single-quote splice breaks on any value containing a quote or shell metacharacter. These are dev/test credentials so severity is Minor, but the safer pattern is to export the variables in the parent shell and let the backgrounded subshell inherit them — which the code already relies on for OPENSHIFT_NAMESPACE/E2E_HS_NAMESPACE. That would also remove the current asymmetry where OIDC vars are forwarded explicitly but OPENSHIFT_NAMESPACE (needed by the openshift driver's acquire_oidc_token) is only inherited. Confidence: High.

2. [Minor] perf_results_init default latency objects omit the avg key.
   tests/e2e/perf/lib.sh:404-405 initialize PERF_RES_CREATE_JSON/PERF_RES_TTR_JSON to {"p50": null, ...} without avg, whereas the module-level defaults (lines 283-284), the documented schema, and completed-run writes all include avg. A run that is interrupted before scale-up metrics are computed therefore writes a history file whose time_to_running_seconds object has no avg. This happens to render as - (matching the spec's "Partial Run Shows Dashes" scenario, so it is harmless in practice), but the two init strings should include "avg": null for schema consistency. Confidence: High.

3. [Minor] Nondeterministic seed selection.
   e2e_discover_seed_ids / e2e_json_first_id (tests/e2e/lib.sh) bind to the first fleet/cluster/release returned by the API. That is correct for the single-seed dev/CI environment the harness targets, but on any environment with more than one fleet/cluster/release the harness silently binds to an arbitrary one determined by API ordering. Consider selecting by a known name or documenting the single-seed assumption at the discovery call site. Confidence: Medium.

Cross-PR coordination

Two open pull requests require a maintainer decision or a defined merge order relative to this one:

• feat(control-plane): adopt upstream OpenShell Helm chart for gateway deployments #194 (adopt upstream OpenShell Helm chart for gateway deployments) changes the gateway deployment mechanism and renames in-namespace gateway resources (e.g. openshell-ca → openshell-gateway-ca, openshell-server → openshell-gateway-server) while editing the same gateway-verification region of tests/e2e/e2e-openshell.sh that this PR reworks into short/long steps. This PR's new OpenShift driver (discover_gateway_endpoint, wait_for_gateway_route) and the performance harness readiness checks assume the current provisioning path and resource names. The two make incompatible assumptions about how a gateway is deployed and what its in-namespace objects are called; whichever merges second must reconcile the resource names and re-validate the harness/driver readiness probes against the Helm-based deployment. Both PRs also rewrite the shared skills/RECONCILE.md checkpoint state, so the coverage/gap tables will need to be reconciled rather than blindly merged. A merge-order decision is needed.

• docs: define control plane reconciliation contract #200 (define control plane reconciliation contract) revises the gateway-deletion and managed-namespace-GC contract in the same specs/platform/e2e-testing.spec.md, restating deletion as finalizer-driven cleanup. This PR adds new normative scenarios and a short-mode "throwaway gateway" GC test premised on the delete-driven GC semantics (accepts 204/404 and polls the namespace to disappearance). The two edits describe the same behavior from different angles; maintainers should confirm which deletion/GC wording is authoritative and align this PR's new short-mode GC assertions with docs: define control plane reconciliation contract #200's finalization model, deciding the merge order accordingly.

Findings Summary (ordered by severity, highest first)

1. [Minor] Credentials interpolated into bash -c worker argv - Security (tests/e2e/e2e-performance.sh)
2. [Minor] perf_results_init default latency objects omit avg - Spec/Schema Consistency (tests/e2e/perf/lib.sh:404-405)
3. [Minor] Nondeterministic seed fleet/cluster/release selection - Robustness (tests/e2e/lib.sh)

Convention Checklist

Convention	Result
Conventional commit messages	Pass
Input validated (E2E_MODE validated, fails fast)	Pass
Reconcile / reuse-or-create pattern (not create-or-skip)	Pass
Test Diff Scrutiny (default long preserves existing assertions)	Pass
No secrets in logs or command arguments	Fail (Minor - argv splice)
Config separate from code (env-driven knobs)	Pass

[jsell-rh]

Verdict

COMMENT (approve-with-nits). This is a well-structured, well-tested local/manual performance harness plus a partial OpenShift e2e driver and an E2E_MODE=short|long split; there is no production Go code, long remains the default so existing CI is untouched, and both cluster-free unit suites (tests/e2e/perf/lib_test.sh, tests/e2e/openshift_driver_test.sh) pass locally. My findings are all Minor (worker credential handling, a small JSON-schema default inconsistency, and nondeterministic seed selection); none block merge, but a couple of cross-PR coordination points below need a maintainer decision.

I reviewed against CLAUDE.md, security.spec.md, control-plane/conventions.spec.md, and the review checklists. I paid particular attention to Test Diff Scrutiny: the pre-existing assertions in tests/e2e/e2e-openshell.sh are gated behind if e2e_step long, and because E2E_MODE defaults to long, every previously-unconditional assertion still runs in the default/CI path — no guarantee is silently removed. The CA-extraction assertion that changed for the openshift branch is a new-driver code path, not a weakened contract for the existing kind path.

Findings

1. [Minor] Credentials spliced into bash -c worker command strings (argv exposure).
   In tests/e2e/e2e-performance.sh, both the scale-up workers (export E2E_OIDC_PASSWORD="'"${E2E_OIDC_PASSWORD}"'" and friends) and the teardown workers build a bash -c '...' string with the secret values interpolated as literals. Those values then appear in the child process argv (visible via ps//proc/<pid>/cmdline) and the single-quote splice breaks on any value containing a quote or shell metacharacter. These are dev/test credentials so severity is Minor, but the safer pattern is to export the variables in the parent shell and let the backgrounded subshell inherit them — which the code already relies on for OPENSHIFT_NAMESPACE/E2E_HS_NAMESPACE. That would also remove the current asymmetry where OIDC vars are forwarded explicitly but OPENSHIFT_NAMESPACE (needed by the openshift driver's acquire_oidc_token) is only inherited. Confidence: High.

2. [Minor] perf_results_init default latency objects omit the avg key.
   tests/e2e/perf/lib.sh:404-405 initialize PERF_RES_CREATE_JSON/PERF_RES_TTR_JSON to {"p50": null, ...} without avg, whereas the module-level defaults (lines 283-284), the documented schema, and completed-run writes all include avg. A run that is interrupted before scale-up metrics are computed therefore writes a history file whose time_to_running_seconds object has no avg. This happens to render as - (matching the spec's "Partial Run Shows Dashes" scenario, so it is harmless in practice), but the two init strings should include "avg": null for schema consistency. Confidence: High.

3. [Minor] Nondeterministic seed selection.
   e2e_discover_seed_ids / e2e_json_first_id (tests/e2e/lib.sh) bind to the first fleet/cluster/release returned by the API. That is correct for the single-seed dev/CI environment the harness targets, but on any environment with more than one fleet/cluster/release the harness silently binds to an arbitrary one determined by API ordering. Consider selecting by a known name or documenting the single-seed assumption at the discovery call site. Confidence: Medium.

Cross-PR coordination

Two open pull requests require a maintainer decision or a defined merge order relative to this one:

• #194 (adopt upstream OpenShell Helm chart for gateway deployments) changes the gateway deployment mechanism and renames in-namespace gateway resources (e.g. openshell-ca → openshell-gateway-ca, openshell-server → openshell-gateway-server) while editing the same gateway-verification region of tests/e2e/e2e-openshell.sh that this PR reworks into short/long steps. This PR's new OpenShift driver (discover_gateway_endpoint, wait_for_gateway_route) and the performance harness readiness checks assume the current provisioning path and resource names. The two make incompatible assumptions about how a gateway is deployed and what its in-namespace objects are called; whichever merges second must reconcile the resource names and re-validate the harness/driver readiness probes against the Helm-based deployment. Both PRs also rewrite the shared skills/RECONCILE.md checkpoint state, so the coverage/gap tables will need to be reconciled rather than blindly merged. A merge-order decision is needed.

• #200 (define control plane reconciliation contract) revises the gateway-deletion and managed-namespace-GC contract in the same specs/platform/e2e-testing.spec.md, restating deletion as finalizer-driven cleanup. This PR adds new normative scenarios and a short-mode "throwaway gateway" GC test premised on the delete-driven GC semantics (accepts 204/404 and polls the namespace to disappearance). The two edits describe the same behavior from different angles; maintainers should confirm which deletion/GC wording is authoritative and align this PR's new short-mode GC assertions with #200's finalization model, deciding the merge order accordingly.

Findings Summary (ordered by severity, highest first)

1. [Minor] Credentials interpolated into bash -c worker argv - Security (tests/e2e/e2e-performance.sh)
2. [Minor] perf_results_init default latency objects omit avg - Spec/Schema Consistency (tests/e2e/perf/lib.sh:404-405)
3. [Minor] Nondeterministic seed fleet/cluster/release selection - Robustness (tests/e2e/lib.sh)

Convention Checklist

Convention	Result
Conventional commit messages	Pass
Input validated (E2E_MODE validated, fails fast)	Pass
Reconcile / reuse-or-create pattern (not create-or-skip)	Pass
Test Diff Scrutiny (default long preserves existing assertions)	Pass
No secrets in logs or command arguments	Fail (Minor - argv splice)
Config separate from code (env-driven knobs)	Pass

[jsell-rh]

[Minor - Security] The scale-up worker interpolates E2E_OIDC_PASSWORD (and the other OIDC vars) as a literal inside the bash -c '...' string, so the secret ends up in the child process argv (ps//proc/<pid>/cmdline) and the single-quote splice breaks on any value containing a quote/metacharacter. The teardown worker (~line 182) does the same. Prefer export-ing these in the parent shell and letting the backgrounded subshell inherit them — as this code already does for OPENSHIFT_NAMESPACE. That also removes the asymmetry where OPENSHIFT_NAMESPACE (needed by the openshift driver's acquire_oidc_token) is only inherited, not forwarded.

[squizzi]

Addressed in 85c7a8c. Scale-up and teardown workers now inherit API_HOST, OIDC, seed ids, and OPENSHIFT_NAMESPACE from the parent via perf_export_child_env. The bash -c strings no longer interpolate secrets into argv.

[jsell-rh]

[Minor - Schema consistency] These init defaults omit the avg key, unlike the module-level defaults (lines 283-284), the documented schema, and completed-run writes, which all include avg. An interrupted run therefore writes time_to_running_seconds with no avg. It renders as - (matching the spec's "Partial Run Shows Dashes" scenario, so harmless), but include "avg": null here for schema consistency.

[squizzi]

Addressed in 85c7a8c. perf_results_init now writes the same {"avg": null, "p50": null, "p90": null, "p99": null, "max": null} objects as the module-level defaults and the schema.