Skip to content

Jeff/fix/autoconf2#1523

Merged
spomichter merged 3 commits intodevfrom
jeff/fix/autoconf2
Mar 11, 2026
Merged

Jeff/fix/autoconf2#1523
spomichter merged 3 commits intodevfrom
jeff/fix/autoconf2

Conversation

@jeff-hykin
Copy link
Member

Problem

I would get the "apply system changes?" every time on MacOS (not just every reboot but every execution)

Solution

Run a command to correctly set the multicast value on systems that already have a value set

Breaking Changes

Should be none, tested on summer's mac and mine.

How to Test

dimos run demo-camera

(should only get asked questions once per reboot)

Contributor License Agreement

  • I have read and approved the CLA.

"""Collect IO counters in bytes. Call inside oneshot()."""
try:
io = proc.io_counters()
io = proc.io_counters() # type: ignore[attr-defined] # not available on macOS
Copy link
Member Author

Choose a reason for hiding this comment

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

my mypy wouldn't pass without this.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 11, 2026

Greptile Summary

This PR fixes a macOS-specific bug where users were prompted to "apply system changes?" on every execution of dimos run rather than only once per reboot. The root cause was that MulticastConfiguratorMacOS.fix() attempted to add the 224.0.0.0/4 multicast route on lo0, but macOS already had a route for that prefix on en0 (created automatically at boot/network-up). The route add silently failed with "route already in use", so the route was never migrated to lo0, causing check() to return False on every subsequent run.

Key changes:

  • lcm.py: MulticastConfiguratorMacOS.fix() now issues a route delete -net 224.0.0.0/4 (with check=False to tolerate no-route scenarios) immediately before the route add -interface lo0, ensuring the delete and re-add are always atomic within the same fix call.
  • test_system_configurator.py: Test updated to assert both the delete and add subprocess.run calls are made and contain the expected arguments.
  • stats.py: Minor # type: ignore[attr-defined] annotation to suppress a type-checker warning for io_counters() being unavailable on macOS.

Confidence Score: 5/5

  • This PR is safe to merge — it addresses a well-understood macOS routing issue with a minimal, targeted fix.
  • The fix correctly identifies the root cause (stale en0 route blocking route add), places the delete atomically before the add within MulticastConfiguratorMacOS.fix(), uses check=False for the delete so missing-route scenarios are handled gracefully, and includes updated tests that verify both subprocess calls. The change was tested on two machines. No regressions are introduced in Linux or buffer configurator paths.
  • No files require special attention.

Important Files Changed

Filename Overview
dimos/protocol/service/system_configurator/lcm.py Adds a route delete -net 224.0.0.0/4 call at the start of MulticastConfiguratorMacOS.fix() before the existing route add, preventing the "route already in use" error that caused the check/fix cycle to repeat on every execution.
dimos/protocol/service/test_system_configurator.py Test updated to assert two subprocess.run calls in fix() — first a route delete (pre-clean), then the route add; assertions verify both call arguments correctly.
dimos/core/resource_monitor/stats.py Trivial: adds # type: ignore[attr-defined] comment to io_counters() call to suppress a type checker warning for the method being unavailable on macOS.

Sequence Diagram

sequenceDiagram
    participant D as dimos run
    participant C as MulticastConfiguratorMacOS.check()
    participant NS as netstat -nr
    participant F as MulticastConfiguratorMacOS.fix()
    participant RD as route delete -net 224.0.0.0/4
    participant RA as route add -net 224.0.0.0/4 -interface lo0

    D->>C: check()
    C->>NS: netstat -nr
    NS-->>C: routing table (224.0.0.0/4 on en0 or missing)
    C-->>D: False (no lo0 route found)

    D->>F: fix()
    F->>RD: sudo route delete -net 224.0.0.0/4 (check=False)
    note over RD: Removes stale en0 route if present.<br/>Silently ignored if no route exists.
    RD-->>F: returncode (ignored)
    F->>RA: sudo route add -net 224.0.0.0/4 -interface lo0 (check=True)
    RA-->>F: returncode=0 (success)

    note over D,RA: Next execution
    D->>C: check()
    C->>NS: netstat -nr
    NS-->>C: 224.0.0.0/4 on lo0 found
    C-->>D: True (no prompt, no fix needed)
Loading

Last reviewed commit: be17a5f

Comment on lines +243 to +253
# Delete any existing 224.0.0.0/4 route (e.g. on en0) before adding on lo0,
# otherwise `route add` fails with "route already in use"
sudo_run(
"route",
"delete",
"-net",
self.MULTICAST_PREFIX,
check=False,
text=True,
capture_output=True,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Route delete placed in the wrong class

The route delete is inside BufferConfiguratorMacOS.fix(), but the corresponding route add -net 224.0.0.0/4 -interface lo0 lives in MulticastConfiguratorMacOS.fix(). These two classes check and fix entirely independent conditions, so they are not guaranteed to both run in the same session.

A concrete failure scenario:

  1. The buffer sysctl values need updating → BufferConfiguratorMacOS.check() returns FalseBufferConfiguratorMacOS.fix() runs and deletes the 224.0.0.0/4 route.
  2. The multicast route is already correct on lo0MulticastConfiguratorMacOS.check() returns TrueMulticastConfiguratorMacOS.fix() is never called.
  3. Result: the valid multicast route has been silently removed, breaking LCM until the next reboot.

The fix should live in MulticastConfiguratorMacOS.fix(), right before the route add, so that the delete and re-add are always atomic:

# in MulticastConfiguratorMacOS.fix()
def fix(self) -> None:
    # Delete any existing 224.0.0.0/4 route (e.g. on en0) before adding on lo0,
    # otherwise `route add` fails with "route already in use"
    sudo_run(
        "route",
        "delete",
        "-net",
        "224.0.0.0/4",
        check=False,
        text=True,
        capture_output=True,
    )
    sudo_run(*self.add_route_cmd, check=True, text=True, capture_output=True)

The route delete was in BufferConfiguratorMacOS.fix(), but the
corresponding route add lives in MulticastConfiguratorMacOS.fix().
If only buffer settings needed updating, the valid multicast route
would be silently deleted without being re-added.
@spomichter
Copy link
Contributor

@greptile

@spomichter spomichter merged commit 4943d15 into dev Mar 11, 2026
10 of 12 checks passed
@spomichter spomichter deleted the jeff/fix/autoconf2 branch March 11, 2026 23:46
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.

3 participants