Skip to content

test(desktop): isolate login-shell probe measurements - #7570

Merged
wesbillman merged 1 commit into
mainfrom
pinky/discovery-probe-test-isolation
Sep 11, 2026
Merged

test(desktop): isolate login-shell probe measurements#7570
wesbillman merged 1 commit into
mainfrom
pinky/discovery-probe-test-isolation

Conversation

@wesbillman

Copy link
Copy Markdown
Collaborator

Pinky, acting on Wes’s behalf.

Summary

Fix a pre-existing false-failure mechanism in the login-shell discovery tests, independently of the thread-window work.

  • Run the two process-global-counter assertions in separate, bounded libtest processes. The global counter still sees all of each test’s auth/worker threads; no production discovery or cache behavior changes.
  • Require exact test selection, successful child exit, and a completion receipt after the assertion body. Reject counter resets outside isolation.
  • Keep the cold/repeated cheap-discovery zero assertions and forced-discovery positive control unchanged. Add parent-probe exclusion and own-worker controls.

Three test-only files, +103/-4. Base 00209076c7a10d9e4a475466c313e8ebecf041f5; head 07f8e86c58a0aa5378537a3daa4610361fdbad55.

Reproduction and causal checks

On the main-based source, cheap discovery alone counted 0. Running the existing missing-binary version test on another thread inside the measurement interval made the repeated-cheap assertion fail with 1. This reproduces interference, not the exact scheduling/caller behind the original full-suite failure. The counter measures run_in_login_shell invocations, not exact OS process counts.

Mutation checks:

  • Removing isolation fails the measurement guard.
  • Removing the real production record() call fails the original forced positive control (0).
  • A reviewer-identified early-return mutation initially survived; the final helper owns the assertion closure instead. Deleting that invocation now fails the external worker assertion (0 vs 1).

Independent read-only review by Brain found no remaining blocking finding at the final head.

Validation at the final head

  • Full just ci: passed. Desktop Tauri: 3,175 passed, 19 ignored; desktop JS: 6,488 passed; mobile: 2,098 passed. Remaining Rust unit, formatting/lint/static, and desktop/web build lanes passed. An initial five-minute tool timeout interrupted compilation; the subsequent complete run used a longer deadline, not altered gates.
  • Normal pre-commit and pre-push hooks: passed. Push includes default + mesh Tauri Clippy, complete Tauri workspace tests, size and branch-scope checks. No bypass.
  • Real local relay, following TESTING.md: passed. Built release buzz-relay, buzz, and buzz-admin from this head. Fresh isolated Postgres/Redis, additive migrations, dedicated loopback ports, disposable NIP-98 identity; health, create channel, signed root/reply send, exact-ID get, and thread retrieval passed. Relay and both owned containers stopped afterward. Dev auth-token mode follows the guide; not a production-auth or thread-window/native UI acceptance claim.

Scope and limits

This is process/cache isolation, not a filesystem sandbox. The existing bounded-command helper bounds child runtime/output; its Unix process-group behavior does not guarantee ownership of nested probe groups. Existing individual probe deadlines remain unchanged.

No UI change, screenshots, thread-window commits, feature enablement, merge, or deployment included.

Originating conversation: buzz://message?channel=0b73400d-6922-4fe3-9b97-454da04d011b&id=984528c883faa8829971d19394870f0f2d543d5c1950565fd55ffddc1b8d8022

Run the counter-owning discovery tests in bounded, exact-filtered libtest processes. Keep process-wide counting so auth workers remain covered, require a completed assertion body, and reject unisolated resets. Add parent-isolation and own-worker controls without changing production discovery.

Signed-off-by: Pinky <5f5ab050ec58ae208332edd544ebf705221e24c1b86d82a6ca07038a7a8f6ac9@buzz.block.builderlab.xyz>
@wesbillman
wesbillman requested a review from a team as a code owner September 10, 2026 23:07
@github-actions

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is 092c6a7277698bd373ccbc1d008fc1507094ae74...07f8e86c58a0aa5378537a3daa4610361fdbad55.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 07f8e86c58a0aa5378537a3daa4610361fdbad55 to authorize a new review.
Any previous review applies only to its recorded range.

@wesbillman wesbillman 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.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Verdict

No actionable code blockers found in this test-isolation change. This is a review comment, not an approval.

Reviewed head 07f8e86c58a0aa5378537a3daa4610361fdbad55 against merge base 00209076c7a10d9e4a475466c313e8ebecf041f5; current PR base was 092c6a7277698bd373ccbc1d008fc1507094ae74. The intervening base changes do not touch desktop/src-tauri.

  • Scope and measurements: all three changed files are test-only. Both original assertion bodies, the cold/repeated zero checks, and the forced positive control are preserved. The counter remains process-wide and attached to the real login-shell invocation path, including joined auth/worker threads.
  • Subprocess boundary: exact libtest selection, child success, and a receipt emitted after the body prevent empty-selection and failed-child passes. The separate post-closure worker assertion detects omission of the closure invocation. Parent and child locks are process-local; the existing subprocess helper bounds runtime and output.
  • Validation: source/call-path review and git diff --check passed. On macOS, the existing default/system-keyring test artifact ran both isolated discovery tests, both new controls, and the missing-binary probe together with --exact --nocapture --test-threads=8: 5 passed, 0 failed. Checkout head, clean state, and binary SHA-256 were checked before/after. This was artifact-level validation, not a fresh build or an independent rerun of the author’s full CI/mutation campaign.

Runtime artifact: buzz_lib-827fc16670abf059, SHA-256 eea65f2beadbdcd6ec9c783e79f18896ffdc729d07dd99414892b3e050f57fe1.

Residual limits are accurately scoped in the PR: this isolates process state, not the filesystem, and does not strengthen the existing Unix ownership boundary for nested probe process groups. Those limitations do not warrant expanding this test-only repair. GitHub CI was still running at the review snapshot; no merge or deployment readiness is implied.

@jedwards27 jedwards27 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.

:bot: Jude’s code review agent — APPROVE

Reviewed: 092c6a7277698bd373ccbc1d008fc1507094ae74..07f8e86c58a0aa5378537a3daa4610361fdbad55 (exact head 07f8e86c58a0aa5378537a3daa4610361fdbad55)

Risk: medium — this is test-only code, but it controls whether process-global discovery measurements produce trustworthy CI results under parallel execution, bounded subprocesses, and failure paths.

Behavior/contracts traced: parent/worker libtest selection; marker and current-thread binding; recursion exclusion; reset isolation; completion receipt ordering; child exit/panic/early-return behavior; timeout/output/process-group bounds; real production probe instrumentation; full-suite interaction; platform gating; production-scope containment.

Findings: no unresolved author-actionable defect. The exact PR range changes only three Tauri test files, and the counter module remains #[cfg(test)]; production discovery/cache behavior is unchanged. The worker uses exact test selection, binds child recognition to both marker and current libtest name, executes the assertion closure before emitting its receipt, and requires successful child exit plus that receipt. Zero selection, rename/ignore, panic, nonzero exit, timeout, overflow, or skipped body therefore fail closed. The positive control still exercises the real production run_in_login_shell seam.

Author action: none.

Verification owner: CI owns the remaining exact-head Desktop Core and Smoke shard 4 completion. Reviewer/tooling owns any desired independent Windows runtime observation; the new isolation path is Unix-gated.

Validation at matching clean head:

  • Both lanes independently passed the canonical full just desktop-tauri-test: main library 3,175 passed, 19 ignored, with all workspace/integration/doc targets green.
  • Focused discovery/bounded-process slice passed 131/131.
  • Causal mutations removing the assertion-closure invocation and removing the real production record() seam each failed the intended worker control, then restoration returned to a clean exact head.
  • cargo fmt --check and git diff --check passed.
  • Exact-head required checks had no failure at final refresh. Windows Rust and three smoke shards had completed green; Desktop Core and Smoke shard 4 remained pending.

Manual/native evidence: not applicable to a test-only subprocess-isolation change. No UI or production-auth behavior changed or was claimed.

Residual risk: the inherited marker is not cryptographically authenticated, though it must equal the exact current test name and no other repository/CI setter was found in the searched scope. The existing bounded-command helper cannot guarantee ownership of nested Unix processes that deliberately escape their process group; its return remains bounded and this PR does not alter that contract. Pending CI is a named gate confidence gap, not author rework.

@jedwards27 jedwards27 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.

Reviewed exact head 07f8e86c58a0aa5378537a3daa4610361fdbad55 against base 092c6a7277698bd373ccbc1d008fc1507094ae74.

No material defects found. The change is confined to three Tauri test files, with the probe module remaining #[cfg(test)]; production discovery/cache behavior is unchanged. The worker contract fails closed: it selects the exact current libtest name, recognizes child mode only when marker and current test name agree, runs single-threaded, and requires both successful process exit and a receipt emitted only after the assertion closure returns. Zero selection, panic, early failure, recursion, timeout, or bounded-output failure therefore cannot produce a pass.

The positive control reaches the real run_in_login_shell instrumentation seam, while the protected tests retain cold/repeated zero-probe assertions and the forced-path positive assertion. Existing bounded-command handling provides a 300-second deadline, aggregate 1 MiB output cap, child reaping, and Unix process-group cleanup. The inherited nested process-group escape limitation is unchanged and does not undermine bounded parent return.

Verification on the exact clean head:

  • Two independent just desktop-tauri-test runs passed; the main lib target reported 3,175 passed, 0 failed, 19 ignored, with all package targets/doc tests green.
  • A focused discovery slice passed 131/131.
  • Two complementary mutations proved the controls are causal: skipping the child assertion body and removing the production probe record each made the intended control fail; both trees were restored clean.
  • git diff --check and Tauri formatting passed.
  • Exact-head GitHub CI completed with no pending or failed checks, including Desktop Core, Windows Rust/build, macOS build, smoke shards, and integration gates.

Residual confidence gap: no reviewer independently exercised native Windows runtime behavior; the added isolation path is Unix-gated, while exact-head Windows CI is green. No author action requested.

@wesbillman
wesbillman merged commit 9847b09 into main Sep 11, 2026
61 checks passed
@wesbillman
wesbillman deleted the pinky/discovery-probe-test-isolation branch September 11, 2026 15:11
rileycrane pushed a commit that referenced this pull request Sep 12, 2026
* origin/main:
  Configure ACP session scope per agent (#7578)
  refactor(buzz-acp): point agents at buzz --help instead of a command table (#7586)
  feat(buzz-cli): render an agent-friendly command tree in --help (#7584)
  fix(avatars): scale agent squircles from normalized paths (#7307)
  fix(mobile): bind same-name mentions to exact selected identities (#7385)
  fix(desktop): isolate quota backoff and reuse channel discovery rosters (#6998)
  test(desktop): isolate login-shell probe measurements (#7570)
  feat(git): add default-branch management to relay and CLI (#7562)
  fix(acp): integrate the Buzz Pi adapter fork (#7552)
  fix(markdown): align mention chip wrapping (#7501)
  fix(relay): reject presence updates when Redis storage fails (#7532)
  fix(desktop): let inbox title and message author names truncate under narrow panes (#7550)
  fix(buzz-acp): report missing models without retrying (#7538)
  fix(desktop): require a Codex adapter with Astra support (#7427)
  fix(desktop): order unnamed roster members by full canonical npub (#7503)
  fix(mobile): standardize public-key identity display on npub (#7493)
  fix(desktop): npub identity controls across profile, agents, and workflows (#7489)
  fix(desktop): npub identity displays for mention, member, and workflow surfaces (#7495)
  fix(desktop): shared npub identity foundation (canonicalNpub, PubKey gate, strict parser) (#7488)
  fix(mobile): render push notification sender identity as npub (#7494)

Signed-off-by: Sol <478bb5a31222ea2b28a3d1afb8b1d598940628f19c2a87efc3c4b822299eeec6@buzz.block.builderlab.xyz>

# Conflicts:
#	desktop/src-tauri/src/commands/media_download.rs
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