Dedupe Iroh selected-path diagnostics

[azooz2003-bit]

Summary

• retain a selected-path diagnostic only when the redacted path class changes
• preserve transport-session lifecycle rows between duplicate path notifications
• reset selected-path diagnostic state when a diagnostic session is cleared

Verification

• regression test fails on the test-only commit 4a5060a47a because duplicate relay/private notifications are exported
• swift test --package-path Packages/Shared/CMUXMobileCore --filter duplicateSelectedPathNotificationsDoNotExportFalseChanges
• swift test --package-path Packages/Shared/CMUXMobileCore (230 tests)
• tagged cloud build irpded, run https://github.com/manaflow-ai/cmux/actions/runs/29702843263

Scope

This removes diagnostic noise only. It does not infer socket churn or suppress genuine unavailable, relay, private-network, or session-lifecycle transitions.

---

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

---

Summary by cubic

Reduce noise in Iroh path diagnostics by deduping consecutive selected-path updates. Prevents exporting false changes while keeping session lifecycle rows intact.

• Bug Fixes
  • Emit selectedPathChanged only when the redacted path kind actually changes.
  • Preserve transport-session lifecycle events between duplicate path notifications.
  • Reset selected-path dedupe state when a diagnostic session is cleared.
  • Add unit test to ensure duplicates are dropped and lifecycle events remain.

^{Written for commit aa3a6d4. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Prevented duplicate diagnostic path notifications from appearing as false changes in exported diagnostic reports.
  • Preserved accurate path classifications and session lifecycle details in diagnostic event data.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7dc3b45d-9f53-4a22-9b9f-f75742b095ef

📥 Commits

Reviewing files that changed from the base of the PR and between 22f9e5e and aa3a6d4.

📒 Files selected for processing (2)

• Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLog.swift
• Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift

---

📝 Walkthrough

Walkthrough

DiagnosticLog now tracks the latest diagnostic path kind and skips retaining repeated matching selected-path notifications while still counting them as processed. Session clearing resets this state, and a regression test verifies exported events and metadata.

Changes

Diagnostic log deduplication

Layer / File(s)	Summary
Track and filter repeated path notifications Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLog.swift	DiagnosticLog.Store tracks the selected path kind, counts every event, skips duplicate matching events, resets the state on clear, and documents the retention behavior.
Validate exported event ordering Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift	The new test verifies filtered event codes, diagnostic path kinds, and session lifecycle metadata.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• manaflow-ai/cmux#8456: Adjusts when and which selected-path diagnostics are published, affecting the event sequences filtered here.

🚥 Pre-merge checks | ✅ 25

✅ Passed checks (25 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: deduplicating selected-path diagnostics.
Description check	✅ Passed	The description includes summary, verification, and scope, so it is mostly complete despite missing optional template sections.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation	✅ Passed	The production change stays inside the private Store actor; no new MainActor coupling or unsafe Sendable reference state was introduced.
Cmux Swift Blocking Runtime	✅ Passed	No new blocking sync in production Swift; the only polling is bounded test scaffolding using Task.yield.
Cmux Browser Automation Off-Main	✅ Passed	Diff only changes DiagnosticLog.swift; the browser-automation policy applies to TerminalController.swift and ControlCommandExecutionPolicy.swift, so this PR doesn’t touch covered browser commands.
Cmux Expensive Synchronous Load	✅ Passed	Diff only updates DiagnosticLog deduping state; no added/moved synchronous agent-history or JSON/load path appears in changed Swift code.
Cmux Cache Substitution Correctness	✅ Passed	DiagnosticLog only keeps the last observed selectedPathKind to dedupe duplicate selectedPathChanged events; cold init and clear reset are handled, and no authoritative read was swapped out.
Cmux No Hacky Sleeps	✅ Passed	Only a Swift source file changed; the diff adds no fixed sleeps/timers/polling, and the rule targets non-Swift runtime scripts.
Cmux Algorithmic Complexity	✅ Passed	Changed append/clear stay O(1): one selectedPathKind comparison plus bounded ring writes; no new scans/sorts/filtering in production code.
Cmux Swift Concurrency	✅ Passed	Diff only adds actor-side selectedPathKind dedupe and async tests; no DispatchQueue, Combine, completion-handler APIs, or new fire-and-forget Tasks were introduced.
Cmux Swift @Concurrent	✅ Passed	Patch only adds actor-isolated selectedPathKind dedupe inside Store; no @concurrent/nonisolated async annotations or UI-isolated heavy async call sites changed.
Cmux Swift Package Boundaries	✅ Passed	The diff is confined to the shared CMUXMobileCore SwiftPM library and its tests, not the app target, so it does not violate the package-boundary rule.
Cmux Swiftpm Lockfiles	✅ Passed	Diff only changes DiagnosticLog.swift and its test; no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project files are touched.
Cmux Swift Logging	✅ Passed	The diff only changes DiagnosticLog deduping and adds a test; no print/debugPrint/dump/NSLog, file-scoped Logger, or secret-bearing logs were introduced.
Cmux User-Facing Error Privacy	✅ Passed	The diff only changes internal diagnostic retention logic and tests/docs; it adds no user-facing errors, alerts, API bodies, or sensitive upstream details.
Cmux Full Internationalization	✅ Passed	PASS: The only production diff is internal DiagnosticLog logic and a doc comment; no user-facing Swift text, catalogs, or locale files were added. Tests are exempt.
Cmux Swiftui State Layout	✅ Passed	The only changed file is a non-UI DiagnosticLog actor; no SwiftUI state/layout patterns from the rule are introduced or expanded.
Cmux Architecture Rethink	✅ Passed	The change keeps the invariant inside DiagnosticLog.Store, adds no new timing/observer path, and the only polling is test-only synchronization.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PR only changes DiagnosticLog.swift; no NSWindow/NSPanel/WindowGroup or cmuxAuxiliaryWindowIdentifiers code is introduced or modified, so the rule is inapplicable.
Cmux Source Artifacts	✅ Passed	The only changed path in HEAD is a hand-written Swift source file; no logs, caches, build output, temp dirs, or other source-control artifacts were added.
Cmux No Test Or Debug Seam In Production Source	✅ Passed	No #if DEBUG/test-only seam or test-named accessor was added; the diff only adds private selectedPathKind state and dedup logic inside Store.
Cmux No Ambient Global State	✅ Passed	No new file-scope API or singleton was added; the new state is a private actor property inside DiagnosticLog.Store.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch task-iroh-path-diagnostic-dedupe

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Greptile Summary

This PR adds deduplication of selectedPathChanged diagnostics in DiagnosticLog's Store actor: consecutive events for the same DiagnosticPathKind are counted but not retained in the ring, while non-path events (e.g. transportSessionLifecycle) between identical path notifications are preserved. The session-level state is reset on clear().

• Moves totalProcessed += 1 before the early-return guard so processedCount() correctly reflects every event that reaches append(), including discarded duplicates — this is the right semantics for the test-drain barrier.
• Adds selectedPathKind: DiagnosticPathKind? to the Store actor and resets it in clear(), matching the PR's third claim in the description.
• The regression test covers the interleaved-lifecycle-between-duplicates scenario and validates processedCount works as a barrier for deduplicated events.

Confidence Score: 4/5

Safe to merge; the dedup logic is correct, actor isolation is maintained, and the drain-barrier semantics are preserved.

The implementation is clean: totalProcessed is correctly incremented before the early return so processedCount() remains a reliable test barrier; selectedPathKind is reset in clear(); non-path events between duplicate notifications are correctly preserved. The one gap is that the selectedPathKind = nil reset on clear() — explicitly called out in the PR description — has no test. A regression there would not be caught by the current suite.

The test file could use one additional case exercising selectedPathChanged across a clear() boundary to cover the reset behaviour the PR describes.

Important Files Changed

Filename	Overview
Packages/Shared/CMUXMobileCore/Sources/CMUXMobileCore/DiagnosticLog.swift	Adds selectedPathKind dedup state to the Store actor; correctly resets it in clear() and counts all events (including discarded) in totalProcessed. The dedup logic is sound; one untested invariant (reset after clear) is noted.
Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift	New test correctly validates the main dedup contract and the processedCount barrier for deduplicated events; does not cover the selectedPathKind = nil reset on clear() that the PR description explicitly claims.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[record event] --> B[Ingress.record\nyield onto event stream]
    B --> C{Stream buffer full?\n.bufferingNewest drops oldest pending}
    C -- no --> D[drain task: Store.append]
    C -- yes --> E[oldest pending dropped\nnever the caller]
    D --> F[totalProcessed += 1]
    F --> G{event.diagnosticPathKind\nnon-nil?}
    G -- no, not a path event --> H[write to ring slot\nadvance head]
    G -- yes --> I{nextPathKind == selectedPathKind?}
    I -- yes, duplicate --> J[early return\nevent discarded\ntotalProcessed still incremented]
    I -- no, changed --> K[selectedPathKind = nextPathKind\nwrite to ring slot]
    L[clear command] --> M[Store.clear\nwipe slots, reset head/filled\ntotalProcessed = 0\nselectedPathKind = nil]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[record event] --> B[Ingress.record\nyield onto event stream]
    B --> C{Stream buffer full?\n.bufferingNewest drops oldest pending}
    C -- no --> D[drain task: Store.append]
    C -- yes --> E[oldest pending dropped\nnever the caller]
    D --> F[totalProcessed += 1]
    F --> G{event.diagnosticPathKind\nnon-nil?}
    G -- no, not a path event --> H[write to ring slot\nadvance head]
    G -- yes --> I{nextPathKind == selectedPathKind?}
    I -- yes, duplicate --> J[early return\nevent discarded\ntotalProcessed still incremented]
    I -- no, changed --> K[selectedPathKind = nextPathKind\nwrite to ring slot]
    L[clear command] --> M[Store.clear\nwipe slots, reset head/filled\ntotalProcessed = 0\nselectedPathKind = nil]

Loading

Comments Outside Diff (1)

1. Packages/Shared/CMUXMobileCore/Tests/CMUXMobileCoreTests/DiagnosticLogTests.swift, line 405-429 (link)

   selectedPathKind reset after clear() is untested

   The PR description lists "reset selected-path diagnostic state when a diagnostic session is cleared" as a distinct delivered behaviour, and the implementation correctly sets selectedPathKind = nil in Store.clear(). However, none of the existing clear tests involve a selectedPathChanged event, so the reset is never exercised. A scenario worth covering: record selectedPathChanged → relay, call clear(), then record another selectedPathChanged → relay — that second event must be retained (not deduplicated) because the dedup state was wiped. Without the = nil assignment the second relay event would be silently discarded, and no existing test would catch the regression.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

_{Reviews (1): Last reviewed commit: "fix(iroh): dedupe selected path diagnost..." | Re-trigger Greptile}