Test: Run the praxis-mode end-to-end checks through cortex-gateway by huang195 · Pull Request #1766 · rossoctl/cortex

huang195 · 2026-11-10T05:50:48Z

Stacks on #1765 (#1765), PR P2 of 3. This branch is built on feat/praxis-hop-1721-pP2. That branch is on a fork, so GitHub cannot use it as the base, and the base is main. Until #1764 and #1765 merge, the diff also shows their commits. Review only the commits after d33ba5fc:

f5ec776a: run the praxis-mode end-to-end checks through cortex-gateway

bf2a33c4: use the praxis toolchain in the end-to-end CI step

50acfde2: fix the plan's gate line and the Codex 405 row text

33bfcc9e: make the end-to-end checks block and compile on Go changes

3185cfb1: fail on a wrong gateway path, and check that the 405 row has no message

What changed

PR P3 of 3 for #1721 (plan: docs/superpowers/plans/2026-11-09-praxis-mode-4-hop-and-egress.md, Task 8).

cmd/cortex/praxis_e2e_test.go (build tag praxis_e2e): 11 TestPraxisE2E_* tests. Each test starts the real cortex-gateway binary that CORTEX_GATEWAY_BIN names. A test dialer sends each origin address to one mock origin, so no name goes to DNS. The tests cover:
- host and cost rows that agree with direct mode, and the same events as direct mode;
- gzip SSE that arrives as frames, and a decoded Accept-Encoding;
- a client hang-up (client_canceled), silence in a stream, and bodies over 10 MiB in each direction;
- fail closed at startup, a Codex response with a wrong SSE label (405 row), no hop header at the origin, and a request with no token to :47605.
.github/workflows/praxis.yaml: a new step builds cortex-gateway with the praxis toolchain and runs TestPraxisE2E with the local profile tags. The trigger also includes hop*.go, egress*.go, cmd/cortex/praxis*.go, cmd/cortex/gateway.go and cmd/cortex/ingest.go. A failure blocks the PR when the PR changes praxis/, the workflow, or one of those Go files.
.github/workflows/ci.yaml: the macOS local-profile step vets cmd/cortex with the praxis_e2e tag, so a change to a helper that the end-to-end file calls fails CI.
Plan: the gate line uses cargo fmt --all --check.

How it was tested

On HEAD 33bfcc9e: all 11 TestPraxisE2E tests pass (SameEventsAsDirect 3 times), with RUSTUP_TOOLCHAIN=1.96.0 and rustup's cargo first on PATH.
go vet -tags praxis_e2e,<local tags> for cmd/cortex, and the other gate steps, pass.
cargo fmt --all --check --manifest-path praxis/Cargo.toml passes.

Size

Counted: 830 lines (830 added, 0 removed), in 2 files. Tests are included.
Exempt: 4 lines (2 added, 2 removed), in the plan document.

Review findings not fixed

[minor] Gate: cargo fmt --check --manifest-path praxis/Cargo.toml fails with "Failed to find targets" (toolchain 1.96.0). cargo fmt --all --check passes. CI is not affected: its fmt step runs cargo fmt --check with working-directory: praxis. From the repo root, the cargo steps also need PATH=$HOME/.cargo/bin:$PATH RUSTUP_TOOLCHAIN=1.96.0. The plan's Task 8 Step 2 command also needs --all.
[nit] .github/workflows/praxis.yaml: if rust-toolchain.toml changes format, sed gives an empty RUSTUP_TOOLCHAIN. Not fixed: the cargo step then fails at once, so the cause is easy to find.
[suggestion] core/listener/forwardproxy/server.go, record.go, tunnel.go and cmd/cortex/main.go do not make an end-to-end failure block the PR. Not fixed: many PRs that are not about praxis mode change these files, and the end-to-end step is new. record.go and tunnel.go now start the workflow. main.go does not.

Fixes #1721

SameEventsAsDirect now fails when ttfbMs or durationMs is on one side only. The origin waits 5 ms, because both fields are omitempty and a 0 ms value has no key.

The tests skip only when CORTEX_GATEWAY_BIN is not set. A set path that names no file fails, so CI cannot pass with no test run.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

coderabbitai · 2026-11-10T05:50:56Z

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 21 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 78d9e259-95db-4548-adfa-8b6ad7bf7d20

Commits

Reviewing files that changed from the base of the PR and between 2cbf8c4 and 9c933eb.

Files selected for processing: 4

.github/workflows/ci.yaml
.github/workflows/praxis.yaml
cmd/cortex/praxis_e2e_test.go
docs/superpowers/plans/2026-11-09-praxis-mode-4-hop-and-egress.md

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

clawgenti

Summary

I reviewed only the three commits after 1c2e62fe, as the description asks: the e2e harness, the CI step and the plan fix. The harness follows main's wiring closely: it reuses startPraxis, outboundEntries, ingestOptions and writeGatewayFiles, so it tests the production code rather than a copy of it. The 11 tests line up with Task 8 of the plan. I found nothing blocking. I left two suggestions and one nit on test strength and CI coverage.

Verification (head 0c33a548, run locally as a non-root user):

cargo build --locked -p cortex-gateway and then the exact CI command (go -C cmd/cortex test -count=1 -timeout 15m -tags "praxis_e2e,$TAGS" -run TestPraxisE2E .): 11/11 pass in 89.5 s.
cargo test --locked in praxis/ passes (I ran it twice). go vet -tags praxis_e2e,<local tags> and gofmt are clean.
Plan gate fix: cargo fmt --check --manifest-path praxis/Cargo.toml exits 1 and cargo fmt --all --check ... exits 0, so the --all change is correct.
Probed the 405 row: Error = {Kind:backend_error Code:405 Message:}, which matches the updated plan text.

CI note: Rust (praxis) fails at cargo test, so this PR's CI skipped the new End-to-end (praxis mode) step. The PR did not cause this. The same step also fails on main at the base commit (87f73caf, push run 42753613836) and on #1765. It passes locally, so it looks flaky or specific to the CI environment. I could not fetch the job log to confirm. Once main is green again, please re-run CI so the e2e step runs at least once on a GitHub runner before this merges.

Blocking scope (changes pattern): I agree that this needs a decision, as the description says. My view: report-only is acceptable for now, because the e2e step is new and its timing behavior on GitHub runners is unproven. Once it has been stable for a while, add the Go paths to pattern.

Author: huang195 (MEMBER — maintainer)
Areas reviewed: Go (e2e tests), CI/GitHub Actions, Docs (plan)
Agent/IDE config (.claude/.vscode): none
Commits: 14 commits in the diff (3 in scope), all signed-off: yes
CI status: failing (Rust (praxis) / cargo test, which also fails on main). All other checks pass.

clawgenti · 2026-11-10T06:34:12Z

+		for _, k := range []string{"ttfbMs", "durationMs"} {
+			dv, _ := d[k].(float64)
+			pv, _ := p[k].(float64)
+			if dv-pv > 50 || pv-dv > 50 {


suggestion: The 50 ms check is weaker than it looks. I logged both sides over three runs. Against the local mock origin, each value is 0 to 4 ms. In 2 of the 3 runs, the direct-mode SSE response row had no ttfbMs or durationMs at all: omitempty drops a 0 ms value. So, as written, this check would also pass if the gateway never reported ttfbMs/durationMs. And the strict fix in the description (key on both sides, or on neither) would be flaky, because direct mode omits sub-millisecond values.

A more robust fix: make the origin wait before it sends headers (for example time.Sleep(60 * time.Millisecond) in the handlers this test uses). Then both modes always have non-zero values. Then require both keys on every response row before you compare them within 50 ms. That pins that praxis rows carry the timing fields, and keeps the comparison meaningful.

clawgenti · 2026-11-10T06:34:12Z

+		t.Fatalf("got %d %q, want 405 %q", st, b, detail)
+	}
+	row := lastResponse(t, c.waitResponses(t, 2, 10*time.Second))
+	if row.StatusCode != http.StatusMethodNotAllowed || row.Error == nil || row.Error.Kind != "backend_error" || row.Error.Code != "405" {


suggestion: The plan line this PR updates now says "The row's message stays empty". I confirmed that at runtime (Message:""), but the test does not assert it. One more condition (|| row.Error.Message != "") would pin that the provider's 405 body does not go into the stored row, which is the property the plan text promises.

clawgenti · 2026-11-10T06:34:12Z

+      - "core/listener/forwardproxy/hop*.go"
+      - "core/listener/forwardproxy/egress*.go"
+      - "cmd/cortex/praxis*.go"
+      - "cmd/cortex/gateway.go"


nit: The e2e harness also depends on cmd/cortex/ingest.go (ingestOptions, readGatewayToken) and on core/attribution/**, which holds the X-Cortex-Attribution encoder that the gateway decodes. praxis/filters/tests/attribution_wire.rs also reads core/attribution/testdata/wire.golden. A PR that changes only those files does not start this workflow. Consider adding both paths to the two paths lists. Separately, as you noted in the description, the new setup-go step (line 129) needs cache-dependency-path: cmd/cortex/go.sum. Without it, setup-go looks for a root go.sum, gives a warning and does not cache modules.

Add cmd/cortex/praxis_e2e_test.go (build tag praxis_e2e). Each test starts the real cortex-gateway binary that CORTEX_GATEWAY_BIN names, and wires praxis mode with startPraxis, as main does. A test dialer sends each origin address to one mock origin. A direct-mode twin with inference-parser gives the rows to compare. The tests cover the W1 items 1, 3, 5, 7, 8 and 9, the Codex mislabeled SSE endpoint, Accept-Encoding, the hop headers, a request with no token to praxis.listen, and the rows of both modes. The rows can differ only in invocations, which praxis/README.md (gateway decision 6) gives, and the test pins that the gateway rows have none. The 11 MiB response streams as SSE. A buffered response over 10 MiB is a 502 in direct mode too. The Praxis workflow builds cortex-gateway and runs these tests after cargo test, with the same blocking rule as the other steps. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>

The step runs from the repo root. rustup reads rust-toolchain.toml only below praxis/, so the step used the default toolchain, which can be older than the 1.96 that the crates need. The step now reads the channel from praxis/rust-toolchain.toml and sets RUSTUP_TOOLCHAIN. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>

cargo fmt --check with --manifest-path stops with "Failed to find targets" on the praxis workspace. The gate now uses --all. The Codex 405 row keeps only a provider error type, not the detail text, so the plan now says what the row holds. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>

- ci.yaml: vet cmd/cortex with the praxis_e2e tag in the local-profile step. Before, only praxis.yaml compiled the end-to-end file. A rename in a helper that the file calls (for example readGatewayToken) did not fail any check. - praxis.yaml: add cmd/cortex/ingest.go to the triggers. A failure now blocks the PR also when the PR changes the Go side of praxis mode: the hop, the egress listener, and the praxis, gateway and ingest files in cmd/cortex. Before, a PR that broke the hop got only a warning. - SameEventsAsDirect: fail when ttfbMs or durationMs is on one side only. Before, a missing key read as 0 and passed the 50 ms check. The origin now waits 5 ms, because both fields are omitempty and a local mock answers in less than 1 ms. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>

- newPraxisChain skips only when CORTEX_GATEWAY_BIN is not set. A set path that names no file now fails the test. Before, a wrong path in CI skipped all 11 tests, and the blocking step passed with no test run. - CodexMislabeledSSE: the 405 row must have no error message. The plan says that the provider's text does not go into the store, and now a test checks it. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>

clawgenti

Summary

Reviewed the commits after d33ba5fc as the description asks (f5ec776..3185cfb): the praxis_e2e test file, the new end-to-end step in praxis.yaml, the praxis_e2e vet in ci.yaml, and the plan text fixes. The tests use the real main wiring (outboundEntries, startPraxis, writeGatewayFiles, readGatewayToken, ingestOptions), so they test what ships. A set but wrong CORTEX_GATEWAY_BIN now fails, not skips. On this head, the End-to-end (praxis mode) step ran in blocking mode and passed in about 1m42s, which fits the 65 s silence test, so the tests ran and were not skipped. I found no blocking issues. The suggestions below are about the scope of the trigger and blocking patterns, and about Go module caching.

Author: huang195 (MEMBER, maintainer)
Areas reviewed: Go tests, CI/GitHub Actions, YAML, Docs
Agent/IDE config (.claude/.vscode): none
Commits: 5 commits in scope (17 in the stacked diff), all signed off: yes
CI status: passing (Rust (praxis), including the new E2E step, ran blocking and passed)

clawgenti · 2026-11-10T13:06:41Z

-          pattern='^(praxis/|\.github/workflows/praxis\.yaml$)'
+          # The hop, the egress listener and the gateway wiring are praxis mode's Go side,
+          # not ported code: a change there that breaks the end-to-end tests must block.
+          pattern='^(praxis/|\.github/workflows/praxis\.yaml$|core/listener/forwardproxy/(hop|egress)[^/]*\.go$|cmd/cortex/(praxis[^/]*|gateway|ingest)\.go$)'


suggestion: The blocking pattern covers hop*.go/egress*.go and three cmd/cortex files. Some of the Go side of praxis mode lives elsewhere, though: hop branch points 1 and 2 are in core/listener/forwardproxy/server.go, branch point 5 is in record.go, the awaiting and joined tunnel settlement is in tunnel.go, the startPraxis and NewUpstreamClient wiring is in cmd/cortex/main.go, and the hop-header encoder is in core/attribution/wire.go. A PR that breaks the end-to-end tests through one of those files reports a warning and does not block. server.go at least triggers the workflow. record.go, tunnel.go, main.go and core/attribution/** do not trigger it at all, so the end-to-end tests don't run for them. Consider adding them to the paths: lists and to this pattern, or say in the header comment that these files are left out on purpose.

clawgenti · 2026-11-10T13:06:41Z


+      - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e  # v7.0.0
+        with:
+          go-version-file: cmd/cortex/go.mod


suggestion: As the description already notes, add cache-dependency-path: cmd/cortex/go.sum. The repo root has no go.sum, so setup-go warns "Restore cache failed" and the step downloads modules on every run. Each setup-go step in ci.yaml sets this value.

clawgenti · 2026-11-10T13:06:41Z

+        continue-on-error: ${{ steps.changes.outputs.blocking != 'true' }}
+        run: |
+          set -euo pipefail
+          RUSTUP_TOOLCHAIN=$(sed -n 's/^channel = "\(.*\)"$/\1/p' praxis/rust-toolchain.toml)


nit: If rust-toolchain.toml ever switches to single quotes or adds a comment on the channel line, the sed prints nothing. RUSTUP_TOOLCHAIN is then exported empty, and set -u does not catch an empty value. A [ -n "$RUSTUP_TOOLCHAIN" ] || { echo '::error::no channel in praxis/rust-toolchain.toml'; exit 1; } guard would make that failure clear.

…op-1721-pP3 Signed-off-by: Hai Huang <huang195@gmail.com> # Conflicts: # docs/superpowers/plans/2026-11-09-praxis-mode-4-hop-and-egress.md

…cache Go modules Fixes review: record.go and tunnel.go did not start the workflow Fixes review: the setup-go step had no cache-dependency-path Files: - .github/workflows/praxis.yaml Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>

clawgenti

Summary

I reviewed the five in-scope commits (f5ec776..3185cfb) plus the merge of main at 1de9ade8. #1764 and #1765 have merged, so the diff against main is now this PR's four files only. I built cortex-gateway with toolchain 1.96.0 and ran TestPraxisE2E with the praxis_e2e and local-profile tags. All 11 tests pass, including SilenceMidStream (66 s). A wrong CORTEX_GATEWAY_BIN fails and an unset one skips, as the description says. go vet -tags praxis_e2e,<local tags> . is clean, and the file is gofmt-clean. The blocking regex matches cmd/cortex/praxis_e2e_test.go, gateway.go, ingest.go, hop*.go and egress*.go, and it does not match gateway_test.go or ingest_test.go, which is what you intended. On this head, CI's End-to-end (praxis mode) step ran in blocking mode and passed in about 1m50s, so the tests really ran. I found no blocking issues. One nit is inline. The open points from the earlier bot review still apply: cache-dependency-path, the scope of the trigger and blocking patterns, and the empty-sed guard. I did not repeat them here.

Author: huang195 (MEMBER — maintainer)
Areas reviewed: Go tests, CI/GitHub Actions, YAML, Docs
Agent/IDE config (.claude/.vscode): none
Commits: 6 commits, all signed-off: yes
CI status: passing

clawgenti · 2026-11-10T14:21:04Z

+func (c *chain) waitReady(t *testing.T, d time.Duration) {
+	t.Helper()
+	deadline := time.Now().Add(d)
+	for !c.hop.Ready() {


nit: If cortex-gateway exits at startup, waitReady only polls hop.Ready(), so every test waits the full 20 s before it fails with "the hop is not ready". I hit this when I ran the suite as root: the gateway prints fatal: Praxis refuses to run as root, exits at once, and all 11 tests take 20 s each before they fail. You can avoid the wait by having startGateway run cmd.Wait() in a goroutine that closes an exited channel, then making waitReady t.Fatalf with the captured output when that channel closes. A failure then names its real cause right away, which helps in containers that run as root by default.

huang195 requested review from a team and rubambiza as code owners November 10, 2026 05:50

rubambiza added this to Rossoctl Issue Prioritization Nov 10, 2026

github-project-automation Bot moved this to New/ToDo in Rossoctl Issue Prioritization Nov 10, 2026

clawgenti approved these changes Nov 10, 2026

View reviewed changes

huang195 added 4 commits November 10, 2026 07:11

huang195 force-pushed the feat/praxis-hop-1721-pP3 branch from 0c33a54 to 33bfcc9 Compare November 10, 2026 12:14

clawgenti approved these changes Nov 10, 2026

View reviewed changes

huang195 added 2 commits November 10, 2026 08:49

Merge commit 'refs/base/praxis-hop-1721-pP3-merge' into feat/praxis-h…

1de9ade

…op-1721-pP3 Signed-off-by: Hai Huang <huang195@gmail.com> # Conflicts: # docs/superpowers/plans/2026-11-09-praxis-mode-4-hop-and-egress.md

huang195 merged commit 28ab6a7 into rossoctl:main Nov 10, 2026
38 checks passed

huang195 deleted the feat/praxis-hop-1721-pP3 branch November 10, 2026 14:09

github-project-automation Bot moved this from New/ToDo to Done in Rossoctl Issue Prioritization Nov 10, 2026

clawgenti approved these changes Nov 10, 2026

View reviewed changes

Navigation Menu

Repository navigation

Test: Run the praxis-mode end-to-end checks through cortex-gateway - #1766

Test: Run the praxis-mode end-to-end checks through cortex-gateway#1766
huang195 merged 7 commits into
rossoctl:mainfrom
huang195:feat/praxis-hop-1721-pP3

huang195 commented Nov 10, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Nov 10, 2026 •

edited

Loading

Review limit reached

Uh oh!

clawgenti left a comment

Uh oh!

clawgenti Nov 10, 2026

Uh oh!

clawgenti Nov 10, 2026

Uh oh!

clawgenti Nov 10, 2026

Uh oh!

clawgenti left a comment

Uh oh!

clawgenti Nov 10, 2026

Uh oh!

clawgenti Nov 10, 2026

Uh oh!

clawgenti Nov 10, 2026

Uh oh!

Uh oh!

clawgenti left a comment

Uh oh!

clawgenti Nov 10, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

huang195 commented Nov 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

How it was tested

Size

Review findings not fixed

Uh oh!

coderabbitai Bot commented Nov 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

clawgenti left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

clawgenti Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

clawgenti Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

clawgenti Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

clawgenti left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

clawgenti Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

clawgenti Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

clawgenti Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

clawgenti left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

clawgenti Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

huang195 commented Nov 10, 2026 •

edited

Loading

coderabbitai Bot commented Nov 10, 2026 •

edited

Loading