Skip to content

fix(egress): record the observed egress address of each address family - #17326

Draft
maxmad64bis wants to merge 1 commit into
cheaper-inference:release/v3.8.53from
maxmad64bis:fix/egress-probe-both-address-families
Draft

maxmad64bis wants to merge 1 commit into
cheaper-inference:release/v3.8.53from
maxmad64bis:fix/egress-probe-both-address-families

Conversation

@maxmad64bis

@maxmad64bis maxmad64bis commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ base-red inherited: #17219

Summary

By default the proxy journal now records the observed address of each address family, so a log line shows the IPv4 and the IPv6 address of the tunnel where it used to show one address with no family. The probe stopped at the first echo target that answered, so a dual-stack tunnel showed its IPv6 address and the IPv4 address of the same node stayed out of the journal. The journal path now queries both targets in parallel and stores each address in two new columns, egress_ip_v4 and egress_ip_v6, next to the unchanged egress_ip. Builds on @adevwithpurpose's dual-target proposal from #9694.

Related Issues

Target Branch

  • Base is the active release/v3.8.x branch (the highest open release/v*), not main
  • New feature? From 3.8.55 on it is held with the v4-feature label for the v4 channel

Validation

  • Change type: DB
  • Focused tests and category gates from the golden path
  • npm run lint
  • Reconciled with the current active release base; focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

  • tests/unit/proxy-echo-dual-family.test.ts (new, 26 cases): both echo targets queried in parallel, the address of each family, the cache merge, the completeness of an observation, the migration and the reconciler.
  • tests/unit/proxy-logger-observed-fields.test.tsx (one case added): the family label and the second observed address in the log detail; no existing assertion changed.

Coverage Notes

  • src/lib/proxyEgress.ts: tests/unit/proxy-echo-dual-family.test.ts.
  • src/lib/proxyEchoTarget.ts: tests/unit/proxy-echo-dual-family.test.ts.
  • src/shared/network/egressAddress.ts: tests/unit/proxy-echo-dual-family.test.ts.
  • src/sse/handlers/proxyJournal.ts: tests/unit/proxy-echo-dual-family.test.ts.
  • src/lib/proxyLogger.ts: tests/unit/proxy-echo-dual-family.test.ts.
  • src/lib/db/schemaColumns.ts: tests/unit/proxy-echo-dual-family.test.ts.
  • src/lib/db/migrations/223_proxy_logs_egress_ip_families.sql: tests/unit/proxy-echo-dual-family.test.ts.
  • src/shared/components/ProxyLogDetail.tsx: tests/unit/proxy-logger-observed-fields.test.tsx.
  • src/shared/components/ProxyLogger.tsx: same shared rule in the egress cell, rendered by tests/unit/proxy-logger-observed-fields.test.tsx.
  • New UI keys are translated in all 67 locale files.

Reviewer Notes

  • Behavior: the journal path now sends 2 echo requests per proxy URL and cache window instead of 1 — delta +1 when the journal measures the URL first, +2 when an operator action already measured the same URL, and delta 0 when the probe already needed both targets anyway (an IPv4-only tunnel: the first target failed, the second answered) or when a single target is pinned — and no request per log line: a complete observation, a failed probe included, is honored until the window ends. The settings routes that test a proxy keep one request and the same answer time, and a null column means the family was not observed, not that the address is missing.
  • Merge: Adds migration 223, the next free number at the base. The migration count in README and its translated mirrors moves with this PR (219 to 220), so check:docs-counts passes; the remaining red entries are inherited from the base, not introduced here. AGENTS.md and llm.txt hunks need the maintainer's explicit OK (agent-instruction files).
  • Follow-up: the unit of comparison stays for a later change (one address, or the /64 IPv6 block). Recording which proxy served each request stays for a later change too: the family stored here is the family of the observed address, not the family of the request that was served. fix(backend): Proxy Test connection times out on IPv4-only SOCKS5/SSH proxies #9694 asked for the probe to try both targets; the journal path does, the settings route that tests a proxy still measures one address.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks, this is a careful piece of work: the pure observation/merge helpers, the monotone cache merge and the UI labels all read well, and I ran proxy-echo-dual-family (26/26), 374 related egress/proxy-log/migration tests and the vitest ProxyLogger case on your head, all passing. Two things before it can merge. (1) The new migration changes the SQL file count from 209 to 210, so npm run check:docs-counts fails (70 drifts) and so do check-docs-counts-sync and readme-migration-count-14574: README.md, AGENTS.md, llm.txt and the 66 README mirrors need the count bumped, like #16087 did for 212. (2) #17332 also claims migration slot 213: whichever lands second should renumber to 214 (check:migration-numbering) and bump the count once more. Note the AGENTS.md/llm.txt count hunks need the maintainer's explicit OK because they are agent-instruction files. The rest of the CI run is still in progress; I will re-check it once it finishes.

@maxmad64bis
maxmad64bis marked this pull request as draft October 21, 2026 19:38
@maxmad64bis
maxmad64bis marked this pull request as ready for review October 21, 2026 20:37
@maxmad64bis

Copy link
Copy Markdown
Contributor Author

Thanks for the review — both points are addressed on the new head (cf88180).

1. The migration count. 87b727c bumps README.md and its 66 docs/i18n README mirrors, cf88180 bumps AGENTS.md, llm.txt and the 66 docs/i18n llm.txt mirrors — 209 → 210 everywhere. npm run check:docs-counts is green again (it reported 70 drifts on the previous head), and check-docs-counts-sync and readme-migration-count-14574 pass with it.

2. The 213 slot. The Merge: bullet of the Reviewer Notes states that this PR keeps 213 and renumbers to 214 (bumping the count once more) if #17332 lands first, so whichever lands second does the renumbering.

The same bullet declares the AGENTS.md and llm.txt hunks as agent-instruction files that need your explicit OK.

One caveat on the CI: on the new head the run is not conclusive on our side — 26 jobs were stopped or cancelled by the runner and 3 are still queued at GitHub, with no test failure attributable to this PR. A re-run from your side would settle it.

@maxmad64bis
maxmad64bis marked this pull request as draft October 24, 2026 01:08
@maxmad64bis
maxmad64bis force-pushed the fix/egress-probe-both-address-families branch from cf88180 to c9b5023 Compare October 24, 2026 01:08
@maxmad64bis
maxmad64bis marked this pull request as ready for review October 24, 2026 09:21
@maxmad64bis
maxmad64bis marked this pull request as draft November 1, 2026 12:22
@maxmad64bis
maxmad64bis force-pushed the fix/egress-probe-both-address-families branch from c9b5023 to 430d752 Compare November 1, 2026 12:22
the egress probe stopped at the first echo target and kept one address, so a log line never said which family it had recorded; the journal path now measures both targets and each line carries the address observed for each family, the reported address always sitting in the column of its own family
@maxmad64bis
maxmad64bis force-pushed the fix/egress-probe-both-address-families branch from 430d752 to 990fd08 Compare November 1, 2026 18:41

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants