Skip to content

fix(pairing): tolerate /proc/net/tcp rows printed twice or missed - #3451

Merged
matt2e merged 2 commits into
mainfrom
mtoohey/flake-peer-proc-net-tcp
Nov 14, 2026
Merged

fix(pairing): tolerate /proc/net/tcp rows printed twice or missed#3451
matt2e merged 2 commits into
mainfrom
mtoohey/flake-peer-proc-net-tcp

Conversation

@matt2e

@matt2e matt2e commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Why

The Rust test credential_service::peer::tests::names_a_separate_process failed nine times in the "Rust and tool integration" job on 9 and 10 November, on unrelated PRs. Every failure was left: None from the Linux peer lookup: five at the helper-side lookup and four at the self-side one.

Assertion Sightings
Helper-side lookup returned None 9 Nov, 10 Nov, 10 Nov, 10 Nov, 10 Nov
Self-side lookup returned None 9 Nov, 9 Nov, 10 Nov, 10 Nov

The failures analysed in depth ran during the larger-runner trial (#3090, reverted by #3180), which gave libtest 16 threads instead of 4. The test is racy in its own right; the runner only raised the odds. The first commit's message counts the four failures that were analysed in depth.

Root cause

The Linux lookup reads live /proc state, which is not a consistent snapshot, while sibling tests in the same binary run alongside it. A standalone copy of the lookup showed two mechanisms:

  • A row printed twice. The kernel writes /proc/net/tcp in pieces. While other sockets open and close, a read across a piece boundary can print a row twice. inode() treated two matching rows as two sockets even though both carried the same inode.
  • Two holders. When a sibling test spawns a process, the child holds copies of every descriptor until it execs, so the uniqueness check saw two holders of the test's socket.

The first is a product defect in the pairing peer lookup on Linux: a real caller can be refused when the table happens to repeat its row. The second is a property of the test's process.

Fix

All in src-tauri/src/credential_service/peer.rs.

  • inode() accepts matching rows that all carry one inode as that one socket, and still names nobody for several distinct inodes. It now reports a missing row separately from an ambiguous one.
  • inode_of() reads the table again, 1 ms apart, while it has no row and the deadline has not passed. The deadline is the existing lookup budget, so a peer that has already gone costs at most the 500 ms the lookup was always allowed.
  • Two holders are still not retried. A genuinely inherited descriptor is meant to name nobody, and the uniqueness check is unchanged.
  • names_a_separate_process re-execs the test binary to run its former body alone with --test-threads=1, as the file already does for its helper. The outer test checks the child's status and its "1 passed" summary, so a renamed path cannot pass vacuously. No sibling test can fork in that process while the sockets are scanned.

The second commit is a review follow-up to a new unit test. It asserted more than one read for a table that never has the row, which needs the first read to land inside a 50 ms window. It now asserts only the outcome there: None, returned no earlier than the deadline. That still fails if the retry is dropped, and the deterministic three-read case proves the re-read.

Tests

  • proc_net_tcp_matches_the_whole_connection gains the repeated-row case and asserts the missing-row result.
  • a_missing_row_is_read_again drives inode_of with a reader that returns the row on its third read, one that never has it, and one with two sockets.

Evidence

The branch-local check was not run: no cargo test or cargo clippy ran on this branch's head, and the local pre-push clippy check was skipped. The same change passed on the combined branch these two commits were split from, and peer.rs here is byte-identical to that branch. Hosted CI on this PR is the first run of this exact tree.

Combined branch, Linux, 32-core host, from the repository root with ./bin/cargo test -p Buzz --lib --locked:

Scope Result
credential_service::peer -- --test-threads=16, five runs after the follow-up 8 passed, 2 ignored each
names_a_separate_process -- --test-threads=16, 10 runs idle 10/10 passed
Same, 10 runs under synthetic load 10/10 passed
Full lib, --test-threads=16, 5 runs under the same load 5/5, 1099 passed and 4 ignored each
cargo clippy -p Buzz --locked --all-targets -- -D warnings, cargo fmt --all --check clean

The load was a standalone copy of the unfixed lookup: 16 threads spawning processes, 8 churning loopback connections, and its own lookups. Over 15,300 lookups per side it logged 193 rows printed two to five times with a single inode and 2,644 two-holder results. Those are the two mechanisms this change addresses, live on the host while the fixed test passed.

Not reproduced, not run

  • The failure did not reproduce in the real test binary, fixed or unfixed. The mechanism is shown by the standalone copy, and the runs above guard against regression rather than reproduce CI timing.
  • No lookup under load saw a table with no row at all, so the re-read path is shown only by its unit test.
  • Nothing was built or run on this branch's head, as stated above.
  • The macOS build was not run. Its lookup path is untouched; it gains only the re-exec, since the test is compiled for macOS and Linux.
  • No human has tested this yet, so buzz-review-completed is not set.

No open PR touches peer.rs.

🤖 Generated with Claude Code

matt2e and others added 2 commits November 13, 2026 13:42
@matt2e @claude
fix(pairing): tolerate /proc/net/tcp rows printed twice or missed
12b71fb 
names_a_separate_process failed four times in CI, every time with
`left: None` from imp::owner (three at the helper-side lookup, one at
the self-side one), all on the larger runner that gave libtest 16
threads instead of 4. The Linux lookup reads live /proc state, which is
not a consistent snapshot, while sibling tests in the same binary run
alongside it. The triage's standalone copy of imp::owner showed two
mechanisms. The kernel writes /proc/net/tcp in pieces, and while other
sockets open and close a read across a piece boundary can print a row
twice; inode() treated two matching rows as two sockets even though both
carried the same inode. And when a sibling test spawns a process, the
child holds copies of every descriptor here until it execs, so unique()
saw two holders of our socket.

Three changes, all in peer.rs:

- inode() accepts matching rows that all carry one inode as that one
  socket, and still names nobody for several distinct inodes. It now
  returns Err(Missing) when no row matches, distinct from Ok(None) for
  several sockets or a passed deadline.
- inode_of() reads the table again, 1 ms apart, while it has no row and
  the deadline has not passed; imp::owner uses it. The deadline is the
  existing LOOKUP budget, so a peer that has already gone costs at most
  the 500 ms the lookup was always allowed. Two holders are still not
  retried: a genuinely inherited descriptor is meant to name nobody, and
  unique() is unchanged.
- names_a_separate_process re-execs the test binary to run the former
  body as the #[ignore]d names_a_separate_process_inner, alone with
  --test-threads=1, as the file already does for connect_and_wait; the
  outer test checks the child's status and its "1 passed" summary, so a
  renamed path cannot pass vacuously. No sibling test can fork in that
  process while the sockets are scanned.

Unit coverage: proc_net_tcp_matches_the_whole_connection gains the
repeated-row case (one inode printed twice is found; two inodes are
still ambiguous) and asserts the new Missing result for a table without
the row. a_missing_row_is_read_again drives inode_of with a reader that
returns the row only on its third read, one that never has it (the
reads continue until the 50 ms deadline), and one with two sockets
(read once, nobody named).

Verified from the repo root with `./bin/cargo test -p Buzz --lib
--locked`:
- `credential_service::peer -- --test-threads=16`: 8 passed, 2 ignored.
- `names_a_separate_process -- --nocapture --test-threads=16` 10x on an
  idle host: 10/10 (0.06-0.07 s); 10x under the load below: 10/10
  (0.21-0.32 s).
- the full lib `-- --test-threads=16` 5x under the load below: 5/5,
  1099 passed and 4 ignored each, 37.5-38.5 s; the 1-minute load
  average rose from 3.4 to 21 on the 32-core host.
The load was the triage's standalone copy of the unfixed imp::owner
(/tmp/flake/H/repro) looping `peerrepro 16 8 300`: 16 threads spawning
/bin/true, 8 churning loopback connections, and its own helper lookups.
Over 51 batches (15,300 lookups per side) it logged 91 helper-side and
102 self-side `NoInode`, every one a row printed 2-5 times with a single
inode, and 2,644 self-side `Two` from its own spawners' children — the
two mechanisms this change addresses, live on this host while the fixed
test passed. No lookup saw a table with no row at all, so the re-read
path is shown only by its unit test. As in the triage, the failure did
not reproduce in the real test binary, fixed or unfixed; these runs
guard against regression rather than reproduce CI timing. `cargo clippy
-p Buzz --locked --all-targets -- -D warnings` and `cargo fmt --all
--check` are clean.

Not run here: the macOS build, whose pids()/sockets() path is untouched;
it gains only the re-exec, since the test is cfg(macos|linux).

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Signed-off-by: mtoohey <[email protected]>
Signed-off-by: Matt Toohey <[email protected]>
@matt2e @claude
test(pairing): assert only the outcome when the socket row never appears
0b543a5 
a_missing_row_is_read_again asserted reads > 1 for the table that never
has the row, which needs the first read to land inside its 50 ms window;
a thread descheduled that long on a loaded runner sees the deadline
passed on read one and fails. Assert only the outcome there, None and
returned no earlier than the deadline, which still fails if the retry is
dropped, since an immediate None lands before the deadline; the
deterministic reads == 3 case above proves the re-read.

Verified as part of the combined review follow-up, before it was split
by path:
- ./bin/cargo test -p Buzz --lib --locked credential_service::peer --
  --test-threads=16, five runs: 8 passed, 2 ignored each.
- cargo fmt --all --check and cargo clippy -p Buzz --locked
  --all-targets -- -D warnings are clean.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Signed-off-by: mtoohey <[email protected]>
Signed-off-by: Matt Toohey <[email protected]>
@matt2e
matt2e requested a review from a team as a code owner November 13, 2026 03:12
kazemek21
kazemek21 approved these changes Nov 13, 2026
View reviewed changes

@kazemek21 kazemek21 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Kyle reviewed this PR and the Nexus review-tracker notes and approved with no comments.

Posted by Codex (gpt-6.1-sol) on behalf of kylez

wesbillman
wesbillman reviewed Nov 13, 2026
View reviewed changes

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord’s automated source review via Wes’s account.

No actionable findings at head 0b543a502edba8feee9ceeafae7389b2fe86e92d, against base/merge-base 7f03764beb8eb0ddcb4ff9ec0b8bda3e07699716.

Reviewed the same-inode deduplication, missing-row retries, process-holder uniqueness, pairing caller lifecycle and isolated test re-exec. Distinct inodes, multiple holders and deadline-truncated scans still name nobody. Lookup remains display-only: unknown-process rendering and human approval/token binding are preserved. The re-exec retains both identity assertions and checks the child status plus the one-test summary.

Validation: source review only; I ran no code, tests, builds or native pairing. Hosted Rust and tool integration passed on the merge of these exact head/base revisions. Its log shows the changed peer tests passing and the full Tauri library at 1,115 passed, 0 failed, 4 ignored in 47.71 s. CI required also passed; Windows native validation was skipped.

The original intermittent failure, live missing-row retry cost, macOS peer-test execution and human pairing acceptance remain unverified. The 500 ms deadline is cooperative; the existing 550 ms async timeout stops waiting but does not cancel a synchronous OS read, and the test wrapper has no independent process timeout. No hard syscall-duration guarantee is inferred.

@matt2e
matt2e merged commit 39e1cfa into main Nov 14, 2026
46 checks passed
@matt2e
matt2e deleted the mtoohey/flake-peer-proc-net-tcp branch November 14, 2026 00:45
wpfleger96 added a commit that referenced this pull request Nov 14, 2026
@wpfleger96
Merge remote-tracking branch 'origin/main' into duncan/cross-communit…
3cec1f1 
…y-unreads

* origin/main: (24 commits)
  [1/2] feat(agents): add a recommended agent setup (core) (#3344)
  test(browser): survive the live-stream restart a page-clock jump causes (#3442)
  chore(renovate): hold rusqlite below 0.41 for mesh-llm (#3438)
  test(browser): serve the diffs and CLI reload fixtures outside the navigation budget (#3445)
  test(browser): play the draft clip at rate 0 instead of looping it before the Play click (#3446)
  fix(status): reopen a status whose preset deadlines coincide as Today (#3449)
  fix(messages): tolerate a cancelled reaction-name chunk load (#3450)
  fix(pairing): tolerate /proc/net/tcp rows printed twice or missed (#3451)
  test(browser): wait on the signal each test asserts in local-agent-activity and message-navigation (#3444)
  Expose native channel headings to Solo sources (#3523)
  test(browser-host): run the 100,000-channel broker scale check alone in the measurements lane (#3447)
  fix(solo): keep every contributed destination action (#3516)
  fix(identity): report an issuer that keeps rate-limiting an agent as a limit (#2365)
  fix(identity): let a stuck at-launch sign-in copy its link (#2453)
  fix(profiles): open avatar previews on hover in compact density (#3510)
  feat(solo): smooth chat recency reordering (#3502)
  Reject self-mentions that cannot wake another session (#3513)
  feat(status): offer don't clear for every status (#3515)
  Let plugin panels name their tab with panels.useTabTitle (#3251)
  fix(agents): report a conversation wake that fails before its turn starts (#2192)
  ...

# Conflicts:
#	tests/browser/message-management.spec.mjs
wpfleger96 added a commit that referenced this pull request Nov 14, 2026
@wpfleger96
Merge remote-tracking branch 'origin/main' into duncan/clock-skew-2min
7c356a2 
* origin/main: (27 commits)
  [2/2] feat(agents): offer Automatic (Recommended) in Create agent and Identity (#3345)
  fix(solo): preserve and resume unsent chat drafts (#3525)
  fix(builderlab): give refused app sign-ins and Identity links a route to Organization sign-in (#3078)
  fix(agents): say why Import an agent can't import here, and link Manage plugins (#3073)
  [1/2] feat(agents): add a recommended agent setup (core) (#3344)
  test(browser): survive the live-stream restart a page-clock jump causes (#3442)
  chore(renovate): hold rusqlite below 0.41 for mesh-llm (#3438)
  test(browser): serve the diffs and CLI reload fixtures outside the navigation budget (#3445)
  test(browser): play the draft clip at rate 0 instead of looping it before the Play click (#3446)
  fix(status): reopen a status whose preset deadlines coincide as Today (#3449)
  fix(messages): tolerate a cancelled reaction-name chunk load (#3450)
  fix(pairing): tolerate /proc/net/tcp rows printed twice or missed (#3451)
  test(browser): wait on the signal each test asserts in local-agent-activity and message-navigation (#3444)
  Expose native channel headings to Solo sources (#3523)
  test(browser-host): run the 100,000-channel broker scale check alone in the measurements lane (#3447)
  fix(solo): keep every contributed destination action (#3516)
  fix(identity): report an issuer that keeps rate-limiting an agent as a limit (#2365)
  fix(identity): let a stuck at-launch sign-in copy its link (#2453)
  fix(profiles): open avatar previews on hover in compact density (#3510)
  feat(solo): smooth chat recency reordering (#3502)
  ...

# Conflicts:
#	src-tauri/src/federated_identity/agent/tests.rs
#	src-tauri/src/federated_identity/mod.rs
#	src-tauri/src/federated_identity/protocol.rs
joahg added a commit to joahg/buzz-app that referenced this pull request Nov 14, 2026
@joahg
Merge remote-tracking branch 'origin/main' into joah/agents-solo-not-dm
0a821dc 
* origin/main: (133 commits)
  Expose ordinary chats without workspace headings (block#3530)
  [2/2] feat(agents): offer Automatic (Recommended) in Create agent and Identity (block#3345)
  fix(solo): preserve and resume unsent chat drafts (block#3525)
  fix(builderlab): give refused app sign-ins and Identity links a route to Organization sign-in (block#3078)
  fix(agents): say why Import an agent can't import here, and link Manage plugins (block#3073)
  [1/2] feat(agents): add a recommended agent setup (core) (block#3344)
  test(browser): survive the live-stream restart a page-clock jump causes (block#3442)
  chore(renovate): hold rusqlite below 0.41 for mesh-llm (block#3438)
  test(browser): serve the diffs and CLI reload fixtures outside the navigation budget (block#3445)
  test(browser): play the draft clip at rate 0 instead of looping it before the Play click (block#3446)
  fix(status): reopen a status whose preset deadlines coincide as Today (block#3449)
  fix(messages): tolerate a cancelled reaction-name chunk load (block#3450)
  fix(pairing): tolerate /proc/net/tcp rows printed twice or missed (block#3451)
  test(browser): wait on the signal each test asserts in local-agent-activity and message-navigation (block#3444)
  Expose native channel headings to Solo sources (block#3523)
  test(browser-host): run the 100,000-channel broker scale check alone in the measurements lane (block#3447)
  fix(solo): keep every contributed destination action (block#3516)
  fix(identity): report an issuer that keeps rate-limiting an agent as a limit (block#2365)
  fix(identity): let a stuck at-launch sign-in copy its link (block#2453)
  fix(profiles): open avatar previews on hover in compact density (block#3510)
  ...

Signed-off-by: Joah Gerstenberg <[email protected]>

# Conflicts:
#	src/bundled/codex/prompt.test.ts
#	src/bundled/goose/runtime.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wesbillman wesbillman wesbillman left review comments
@kazemek21 kazemek21 kazemek21 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@matt2e @wesbillman @kazemek21