feat: add backward-compatible Container support for Agent/Protocol arhitecture#47
feat: add backward-compatible Container support for Agent/Protocol arhitecture#47
Conversation
…chitecture Enables the Container to multiplex both old Base-derived and new Agent/Protocol-derived objects on a shared poll, fixing AttributeError when running ConsulProxyServer. Adds _container_loop bridging, event relay, proxy throttle fixes, and comprehensive test coverage for container setup and proxy data flow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdd container-loop wiring and Base-compatible lifecycle hooks to agents; extend StreamProtocol to proxy connection/socket and flow-control properties; implement client-level protocol event relay; change proxy read-delegation paths; add comprehensive container and reverse-proxy tests; expand architecture/compat docs and update CHANGELOG. Changes
Sequence Diagram(s)sequenceDiagram
actor Observer
participant HTTPClient as HTTP Client
participant ClientAgent as Client Agent
participant Container as Container/_container_loop
participant StreamProtocol as StreamProtocol
participant Connection as Connection/Transport
Observer->>HTTPClient: attach listener
HTTPClient->>ClientAgent: connect(host,port)
ClientAgent->>Container: use _container_loop / compat.connect_stream
Container->>StreamProtocol: create protocol (native/compat)
StreamProtocol->>Connection: bind transport / connection_made
Connection->>StreamProtocol: propagate connection state
StreamProtocol->>ClientAgent: protocol ready
ClientAgent->>HTTPClient: _relay_protocol_events(protocol)
HTTPClient->>Observer: re-emit headers/message/partial/data/open/close
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/netius/base/agent.py`:
- Around line 132-164: The call in Agent.connect unpacks *args after keyword
args into compat.connect_stream which triggers Ruff B026 and can raise TypeError
when positional args are provided; fix by either removing *args from the
compat.connect_stream call (since current usage only passes host, port, ssl) or
by moving *args before the explicit keyword args so positional args are
forwarded correctly, and update the connect signature accordingly; additionally
validate that cls.protocol exists and is callable (or explicitly call a
documented classmethod) by adding a guard that raises NotImplementedError if
cls.protocol is missing/not callable to make the subclass contract explicit
(referencing connect, cls.protocol, and compat.connect_stream).
In `@src/netius/servers/socks.py`:
- Line 231: The change calling self.reads((tunnel_c.socket,), state=False) will
raise AttributeError because SOCKSServer (and its bases ServerAgent/Agent) don't
implement reads()/writes()/errors(); revert to using the original raw_client
interface or add the missing stub methods. Fix option A: change the call back to
self.raw_client.reads((tunnel_c.socket,), state=False) and ensure raw_client is
correctly assigned in SOCKSServer.__init__ (or wherever the client is created).
Fix option B: add no-op stub methods reads(), writes(), and errors() to
Agent/ServerAgent that forward to raw_client if present (or raise a clear
error), so Container can safely call them; update SOCKSServer to use those stubs
consistently.
In `@src/netius/test/extra/proxy_r.py`:
- Line 821: The test sets frontend.ctx_request.return_value =
contextlib.nullcontext(), which fails on Python versions lacking nullcontext;
add a small polyfill before its use (e.g., at module top or before the test)
that detects whether contextlib.nullcontext exists and, if not, defines a
compatible nullcontext context manager (matching the standard library signature)
and assigns it into the contextlib namespace or imports it into the module
namespace; update the test to use the fallback name if needed so
frontend.ctx_request.return_value always gets a working nullcontext across
Python 2.7+ and 3.x.
🧹 Nitpick comments (5)
src/netius/base/container.py (1)
181-186: Duplicate_container_loopassignment acrossstart_baseandapply_base— intentional but worth a note.Both
start_base(line 163) andapply_base(line 185) set_container_loopfor non-Baseobjects. This redundancy is safe but could become a maintenance concern if the logic diverges. Consider extracting into a small helper if more attributes are added in the future.src/netius/test/extra/proxy_r.py (1)
806-827: Consider making_make_frontendmore robust for attribute deletion.The
del frontend.prefix/del frontend.stateetc. pattern works withMagicMockto simulatehasattrreturningFalse. This is a valid technique but somewhat fragile — if the production code changes which attributes it checks, these tests could silently pass with stale mock configuration.src/netius/base/agent.py (1)
166-181: Event relay wiring is clean; minor note on unbinding.The relay correctly maps protocol
"open"→ agent"connect"and protocol"close"→ agent"close", matching the observer trigger signatures. One thing to be aware of: the lambdas are anonymous, so there's no way tounbindthem if a protocol is discarded before it fires events. If protocols are always short-lived and cleaned up, this is fine; otherwise leaked bindings could accumulate.src/netius/test/base/container.py (2)
36-44: Good test structure with setUp/tearDown lifecycle management.The fixture properly creates and cleans up the container. Note that a few tests (e.g.
test_cleanup_agentat line 183) callself.container.cleanup()explicitly, and thentearDowncalls it again. Verify that double-cleanup onContaineris idempotent to avoid spurious test failures.#!/bin/bash # Check if Container.cleanup() is safe to call twice ast-grep --pattern $'class Container($$$) { $$$ def cleanup(self) { $$$ } $$$ }' # Also check Base.cleanup for idempotency rg -n "def cleanup" --type=py -B 1 -A 15 -g '*/base/*.py'
212-260: Event relay tests validate the protocol → client → observer pipeline effectively.Both the HTTP and Raw relay tests cover the full event chain: protocol events are triggered, relayed through the client agent, and received by bound observers. The HTTP test also correctly distinguishes
"headers","message", and"partial"bindings (lines 216-218) even though those events aren't triggered in this particular test — consider adding trigger calls for those protocol-specific events to fully exercise the HTTP relay, or add a TODO.
| def connect(self, host, port, ssl=False, *args, **kwargs): | ||
| """ | ||
| Creates a new protocol based connection to the provided | ||
| host and port, using the container's shared event loop when | ||
| available (dual architecture support). | ||
|
|
||
| :type host: String | ||
| :param host: The hostname or IP address of the remote | ||
| host to which the connection should be made. | ||
| :type port: int | ||
| :param port: The port number of the remote host to | ||
| which the connection should be made. | ||
| :type ssl: bool | ||
| :param ssl: If the connection should be established using | ||
| a secure SSL/TLS channel. | ||
| :rtype: Protocol | ||
| :return: The protocol instance that represents the newly | ||
| created connection. | ||
| """ | ||
|
|
||
| cls = self.__class__ | ||
| protocol = cls.protocol() | ||
| compat.connect_stream( | ||
| lambda: protocol, | ||
| host=host, | ||
| port=port, | ||
| ssl=ssl, | ||
| loop=self._container_loop, | ||
| *args, | ||
| **kwargs | ||
| ) | ||
| self._relay_protocol_events(protocol) | ||
| return protocol |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the compat module and check connect_stream signature
find . -type f -name "*.py" | head -20
git ls-files | grep -E "(compat|agent)" | head -20Repository: hivesolutions/netius
Length of output: 719
🏁 Script executed:
# Look at the file being reviewed
cat -n src/netius/base/agent.py | sed -n '132,164p'Repository: hivesolutions/netius
Length of output: 1456
🏁 Script executed:
# Search for connect_stream definition
rg -n "def connect_stream" --type=py -A 10Repository: hivesolutions/netius
Length of output: 700
🏁 Script executed:
# Search for _connect_stream implementations
rg -n "_connect_stream" --type=py -A 5 | head -50Repository: hivesolutions/netius
Length of output: 1126
🏁 Script executed:
# Check for ClientAgent subclasses and protocol definitions
rg -n "class.*Agent\(" --type=py -B 1 -A 5 | head -100Repository: hivesolutions/netius
Length of output: 1069
🏁 Script executed:
# Search for protocol classmethod/staticmethod definitions
rg -n "def protocol" --type=py -B 2 -A 5Repository: hivesolutions/netius
Length of output: 46
🏁 Script executed:
# Try to parse the file as Python to see if it's syntactically valid
python3 -m py_compile src/netius/base/agent.py 2>&1 || echo "Parse error found"Repository: hivesolutions/netius
Length of output: 46
🏁 Script executed:
# Get full signatures of the implementation functions
cat -n src/netius/base/compat.py | sed -n '604,625p'
cat -n src/netius/base/compat.py | sed -n '680,701p'Repository: hivesolutions/netius
Length of output: 1349
🏁 Script executed:
# Search for calls to connect() method
rg -n "\.connect\(" --type=py -B 2 -A 2 | head -60Repository: hivesolutions/netius
Length of output: 3391
🏁 Script executed:
# Check if any subclass defines protocol classmethod or method
rg -n "class.*ClientAgent" --type=py -A 50 | grep -E "(class|def protocol)" | head -30Repository: hivesolutions/netius
Length of output: 1218
🏁 Script executed:
# Look for protocol classmethod definitions in subclasses
rg -n "def protocol\|protocol =" --type=py -B 3 -A 5 | head -80Repository: hivesolutions/netius
Length of output: 46
🏁 Script executed:
# Check ClientAgent more carefully for protocol definition
cat -n src/netius/base/agent.py | sed -n '106,180p'Repository: hivesolutions/netius
Length of output: 3238
🏁 Script executed:
# Verify the actual syntax is valid and check what the review is referring to
python3 << 'EOF'
# Simulate the current pattern
def connect_stream(protocol_factory, host, port, ssl=False, loop=None, *args, **kwargs):
print(f"protocol_factory: {protocol_factory}")
print(f"host: {host}")
print(f"port: {port}")
print(f"args: {args}")
print(f"kwargs: {kwargs}")
def test_current_call():
"""Test the current call pattern from agent.py"""
def protocol():
return "protocol"
extra_args = ("extra1", "extra2")
extra_kwargs = {"extra_key": "extra_value"}
# This is what agent.py currently does
connect_stream(
lambda: protocol,
host="localhost",
port=8080,
ssl=False,
loop=None,
*extra_args,
**extra_kwargs
)
test_current_call()
EOFRepository: hivesolutions/netius
Length of output: 253
🏁 Script executed:
# Search specifically for "protocol" classmethod/staticmethod in all client files
rg -n "def protocol" --type=py -B 2 -A 3Repository: hivesolutions/netius
Length of output: 46
🏁 Script executed:
# Check a few specific subclass files for protocol() definition
grep -n "def protocol" src/netius/clients/http.py src/netius/clients/dns.py src/netius/clients/raw.py src/netius/clients/ws.py 2>/dev/nullRepository: hivesolutions/netius
Length of output: 46
🏁 Script executed:
# Try broader search in all Python files for protocol as method
rg -n "^\s*def protocol" --type=pyRepository: hivesolutions/netius
Length of output: 46
Move *args before keyword arguments or remove it if positional arguments aren't needed.
Ruff B026 is valid. When *args is unpacked after keyword arguments in the call to compat.connect_stream() (line 160), it causes a mismatch: any positional arguments supplied to connect() would collide with the explicitly named parameters, raising TypeError: got multiple values for argument. Current usage patterns pass only host, port, and ssl, so removing *args aligns with actual usage. If positional arguments are genuinely required for connect_stream, place *args before keyword arguments instead.
Additionally, the cls.protocol() call at line 153 assumes subclasses define a protocol classmethod, but this contract is undocumented. Consider documenting this requirement or raising NotImplementedError if not defined to surface misuse early.
🧰 Tools
🪛 Pylint (4.0.4)
[error] 153-153: Class 'ClientAgent' has no 'protocol' member
(E1101)
🪛 Ruff (0.15.0)
[warning] 160-160: Star-arg unpacking after a keyword argument is strongly discouraged
(B026)
🤖 Prompt for AI Agents
In `@src/netius/base/agent.py` around lines 132 - 164, The call in Agent.connect
unpacks *args after keyword args into compat.connect_stream which triggers Ruff
B026 and can raise TypeError when positional args are provided; fix by either
removing *args from the compat.connect_stream call (since current usage only
passes host, port, ssl) or by moving *args before the explicit keyword args so
positional args are forwarded correctly, and update the connect signature
accordingly; additionally validate that cls.protocol exists and is callable (or
explicitly call a documented classmethod) by adding a guard that raises
NotImplementedError if cls.protocol is missing/not callable to make the subclass
contract explicit (referencing connect, cls.protocol, and
compat.connect_stream).
|
|
||
| tunnel_c.enable_read() | ||
| self.raw_client.reads((tunnel_c.socket,), state=False) | ||
| self.reads((tunnel_c.socket,), state=False) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check ServerAgent definition and reads() method availability
echo "=== Checking ServerAgent definition ==="
rg -n 'class ServerAgent' --type py -A 5
echo "=== Checking Agent class definition ==="
rg -n 'class Agent' --type py -A 5
echo "=== Checking reads method in the codebase ==="
rg -n 'def reads' --type py
echo "=== Checking how ServerAgent is imported ==="
rg -n 'ServerAgent' src/netius/__init__.py -B 2 -A 2
echo "=== Check agent.py for method definitions ==="
cat -n src/netius/agent.py | head -100
echo "=== Check inheritance of SOCKSServer in detail ==="
rg -n 'class SOCKSServer' src/netius/servers/socks.py -A 15Repository: hivesolutions/netius
Length of output: 1831
🏁 Script executed:
#!/bin/bash
# Need to check if Agent has reads() method and understand the full picture
echo "=== Full Agent class definition ==="
cat -n src/netius/base/agent.py
echo "=== Check what ContainerServer is ==="
rg -n 'class ContainerServer' --type py -A 10
echo "=== Check ProxyServer to understand analogous pattern ==="
rg -n 'class ProxyServer' src/netius/servers/proxy.py -A 20
echo "=== Check what HTTP2Server inherits from ==="
rg -n 'class HTTP2Server' --type py
echo "=== Look for any method stubs or monkey-patching in agent.py ==="
sed -n '180,210p' src/netius/base/agent.pyRepository: hivesolutions/netius
Length of output: 9430
🏁 Script executed:
#!/bin/bash
# Check if reads() is added to Agent class anywhere (might be further down)
echo "=== Check full Agent class for reads method ==="
rg -n 'def reads' src/netius/base/agent.py
# Check if Agent has any other Base-compatible stubs we missed
echo "=== Check all method definitions in Agent class ==="
sed -n '38,185p' src/netius/base/agent.py | grep -n 'def '
# Check if reads is added via assignment/setattr
echo "=== Check for dynamic method assignment in Agent ==="
rg -n 'reads.*=' src/netius/base/agent.py
# Let's also check the actual SOCKSServer usage - where does raw_client come from?
echo "=== Check raw_client in SOCKSServer ==="
rg -n 'raw_client' src/netius/servers/socks.py -B 2 -A 2
# Check if there's a reads method that was supposed to be added but wasn't
echo "=== Check Server class reads method signature ==="
sed -n '417,430p' src/netius/base/server.pyRepository: hivesolutions/netius
Length of output: 1867
🏁 Script executed:
#!/bin/bash
# Find raw_client definition in SOCKSServer
echo "=== Find raw_client initialization in SOCKSServer ==="
rg -n 'self.raw_client' src/netius/servers/socks.py -B 2 | head -40
# Look at the full __init__ method
echo "=== Full SOCKSServer __init__ method ==="
sed -n '120,150p' src/netius/servers/socks.py
# Check what RawClient is
echo "=== Find RawClient class ==="
rg -n 'class RawClient' --type py
# Check line 231 in context
echo "=== Context around line 231 ==="
sed -n '225,240p' src/netius/servers/socks.pyRepository: hivesolutions/netius
Length of output: 2340
🏁 Script executed:
#!/bin/bash
# Find where raw_client is defined - check if it's a property or in parent class
echo "=== Search for raw_client property or assignment ==="
rg -n 'raw_client' src/netius/servers/socks.py
echo "=== Check if Container or ContainerServer defines raw_client ==="
rg -n 'raw_client' src/netius/base/container.py
echo "=== Check what add_base does ==="
rg -n 'def add_base' src/netius/base/container.py -A 10Repository: hivesolutions/netius
Length of output: 772
🏁 Script executed:
#!/bin/bash
# Check if raw_client is a property defined in SOCKSServer or parent
echo "=== Search for raw_client as property or descriptor ==="
rg -n '@property' src/netius/servers/socks.py -A 3
# Check Container class to see if it provides raw_client
echo "=== Check Container class full definition ==="
sed -n '1,150p' src/netius/base/container.py | tail -60
# Check if raw_client is created via add_base or elsewhere
echo "=== Check if raw_client is created as attribute ==="
sed -n '130,145p' src/netius/servers/socks.py
# Let's see what RawClient looks like
echo "=== Check RawClient class ==="
sed -n '45,80p' src/netius/clients/raw.pyRepository: hivesolutions/netius
Length of output: 3962
🏁 Script executed:
#!/bin/bash
# The issue is clear: raw_client is referenced but never assigned. Let me check if this exists elsewhere
echo "=== Check if raw_client is dynamically created ==="
rg -n 'raw_client\s*=' src/netius/servers/socks.py
# Check git history or another version to understand raw_client
echo "=== Check all files that reference raw_client ==="
rg -n 'raw_client' --type py
# Let me check if Agent is supposed to have reads() stub but is missing it
echo "=== Look for PR changes or TODOs about Agent stubs ==="
rg -n 'stub' src/netius/base/agent.py -B 2 -A 2
# Check if reads is supposed to be in Agent but just isn't shown
echo "=== Full remaining Agent code after line 104 ==="
tail -n +104 src/netius/base/agent.pyRepository: hivesolutions/netius
Length of output: 6761
🏁 Script executed:
#!/bin/bash
# Critical finding: raw_client is never assigned in SOCKSServer but is referenced
# Let's check if this is a pre-existing bug or part of this change
echo "=== Check ProxyServer's raw_client assignment ==="
sed -n '170,195p' src/netius/servers/proxy.py
echo "=== Compare: SOCKSServer missing raw_client assignment ==="
echo "Lines 120-145 of socks.py show raw_client is used but never assigned:"
sed -n '120,150p' src/netius/servers/socks.py
echo "=== Check if Agent actually has reads() stub that should exist ==="
rg -n 'def reads\|def writes\|def errors' src/netius/base/agent.py
echo "=== What Agent methods are supposed to be Base-compatible stubs? ==="
grep -A 30 'Base-compatible stub' src/netius/base/agent.pyRepository: hivesolutions/netius
Length of output: 3633
🏁 Script executed:
#!/bin/bash
# Final verification: Check if reads/writes/errors stubs should be in Agent
echo "=== Check if there are any recent commits that added stubs to Agent ==="
git log --oneline -20 src/netius/base/agent.py 2>/dev/null || echo "Git log not available"
echo "=== Check Container's poll.poll_owner() to see what it calls ==="
sed -n '110,135p' src/netius/base/container.py
echo "=== Verify: Agent docstring says Base-compatible stubs but shows none ==="
sed -n '38,105p' src/netius/base/agent.py | grep -E '(def |Provides Base)'
echo "=== Let's check if this is part of the current PR changes ==="
echo "Looking for any Agent modifications in the repo..."
git diff HEAD src/netius/base/agent.py 2>/dev/null | head -100 || echo "No git diff available"Repository: hivesolutions/netius
Length of output: 1771
🏁 Script executed:
#!/bin/bash
# The commit message suggests reads/writes/errors stubs should exist
# Let's check the commit to see what was supposed to be added
echo "=== Check that commit for what was added ==="
git show 8f10356 --stat 2>/dev/null | head -50 || echo "Commit details not available"
echo "=== Check if reads/writes/errors are supposed to be in Agent ==="
git show 8f10356 -- src/netius/base/agent.py 2>/dev/null | grep -A 5 -B 5 'def reads\|def writes\|def errors' || echo "No reads/writes/errors found in commit"
echo "=== Current state: Agent missing reads/writes/errors stubs needed by Container ==="
echo "Container calls base.reads(), base.writes(), base.errors() on all bases (line ~127)"
echo "But Agent class (which ServerAgent inherits from) doesn't have these methods"
echo "This is a critical incompleteness in the stub implementation"Repository: hivesolutions/netius
Length of output: 3039
🏁 Script executed:
#!/bin/bash
# Let me verify the exact issue by checking what the reads() signature expects
echo "=== Server.reads() full signature and implementation ==="
sed -n '417,435p' src/netius/base/server.py
echo "=== Confirm: Agent has NO reads/writes/errors stubs ==="
echo "Agent stubs that DO exist:"
sed -n '38,185p' src/netius/base/agent.py | grep 'def ' | head -20
echo "=== Agent stubs that are MISSING (required by Container.poll) ==="
echo "Container calls: base.reads(reads), base.writes(writes), base.errors(errors)"
echo "But Agent has no: def reads, def writes, def errors"
echo "=== Conclusion: raw_client attribute is never assigned in SOCKSServer ==="
echo "Line 143: self.add_base(self.raw_client) <- raw_client doesn't exist!"
echo "Line 147: self.raw_client.destroy() <- raw_client doesn't exist!"
echo "Line 177: self.raw_client.connect() <- raw_client doesn't exist!"
echo "Line 231: self.reads(...) <- reads() doesn't exist on ServerAgent!"Repository: hivesolutions/netius
Length of output: 1916
self.reads() is not available on SOCKSServer—this change will raise AttributeError at runtime.
SOCKSServer inherits from netius.ServerAgent → Agent, neither of which defines reads(). The commit adding backward-compatible Container support (8f10356) promised Base-compatible stubs but never added the reads(), writes(), or errors() methods that Container calls on all bases. The original code used self.raw_client.reads() (though raw_client itself is never assigned in __init__). Either add the missing stub methods to Agent or revert this change to the original approach.
🧰 Tools
🪛 Pylint (4.0.4)
[error] 231-231: Instance of 'SOCKSServer' has no 'reads' member
(E1101)
🤖 Prompt for AI Agents
In `@src/netius/servers/socks.py` at line 231, The change calling
self.reads((tunnel_c.socket,), state=False) will raise AttributeError because
SOCKSServer (and its bases ServerAgent/Agent) don't implement
reads()/writes()/errors(); revert to using the original raw_client interface or
add the missing stub methods. Fix option A: change the call back to
self.raw_client.reads((tunnel_c.socket,), state=False) and ensure raw_client is
correctly assigned in SOCKSServer.__init__ (or wherever the client is created).
Fix option B: add no-op stub methods reads(), writes(), and errors() to
Agent/ServerAgent that forward to raw_client if present (or raise a clear
error), so Container can safely call them; update SOCKSServer to use those stubs
consistently.
There was a problem hiding this comment.
Pull request overview
This PR updates Netius’ Container ecosystem to support multiplexing both legacy Base-derived components and newer Agent/Protocol-based components on the same poll/event loop (targeting issues like the AttributeError seen when running ConsulProxyServer).
Changes:
- Add
Base-compatible lifecycle/introspection stubs toAgentand introduce_container_looppropagation fromContainertoAgentinstances. - Add protocol→client event relaying for
HTTPClient/RawClient, and adjust proxy throttle paths to use the proxy server’s poll registration (self.reads(...)). - Add/expand tests for container setup + reverse proxy request/response flow, and document the change in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 21 comments.
Show a summary per file
| File | Description |
|---|---|
| src/netius/base/agent.py | Adds Base-like stub methods to Agent; adds _container_loop; introduces ClientAgent.connect() and protocol event relaying hooks. |
| src/netius/base/container.py | Sets _container_loop on non-Base objects to force protocol connections onto the container owner’s loop. |
| src/netius/base/protocol.py | Adds backward-compat properties/methods on StreamProtocol to look like legacy Connection objects (socket/throttling/pending limits). |
| src/netius/clients/http.py | Uses _container_loop as default loop when unset; relays protocol events through the client to maintain client-first observer patterns. |
| src/netius/clients/raw.py | Adds protocol event relaying for data via the client. |
| src/netius/extra/proxy_r.py | Updates reverse proxy to handle HTTPClient.method() returning (loop, protocol) in async mode. |
| src/netius/servers/proxy.py | Fixes proxy throttling to register reads on the proxy server (owner of the sockets), not on the internal clients. |
| src/netius/servers/socks.py | Same throttling fix as proxy server (server owns the sockets). |
| src/netius/test/base/container.py | New tests covering container behavior with Agent-based clients and mixed Base+Agent setups. |
| src/netius/test/extra/proxy_r.py | Large expansion of reverse proxy tests for routing, relaying, error handling, and container wiring. |
| CHANGELOG.md | Documents the new dual-architecture container support, event relays, throttle changes, and fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| del frontend.prefix | ||
| del frontend.state | ||
| del frontend.proxy_c | ||
| del frontend.tunnel_c | ||
| return frontend |
There was a problem hiding this comment.
del frontend.prefix/state/proxy_c/tunnel_c on a fresh MagicMock will typically raise AttributeError because those attributes were never set. This makes the test helper brittle and likely to fail before reaching the code under test. Prefer a lightweight real object for frontend (e.g. a custom class / types.SimpleNamespace) or create the mock with spec_set so hasattr() behaves correctly without deleting attributes.
Adds architecture.md covering the event loop, Protocol/Transport, Agent, and Container design. Refocuses compat.md on execution modes, the CompatLoop adapter, and native vs compat performance tradeoffs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@doc/architecture.md`:
- Around line 7-10: Replace the hyphenated phrase "Bi-directional asyncio
Compatibility" (and any occurrences of "bi-directional compatibility") with the
unhyphenated "Bidirectional asyncio Compatibility" everywhere in the document;
update the section header "Bi-directional asyncio Compatibility" and any inline
uses of "bi-directional compatibility" to "Bidirectional" to match standard
technical usage and asyncio wording.
- Around line 129-131: Replace the informal "eg" with the standard abbreviation
"e.g." in the sentence "Connections are owned by the right Base (eg
ProxyServer)" so the line reads "Connections are owned by the right Base (e.g.
ProxyServer)"; update any other occurrences of "eg" in the same document for
consistency and run a quick spell/grammar check on doc/architecture.md to ensure
punctuation spacing matches the style.
In `@doc/compat.md`:
- Around line 69-75: In _connect_stream_compat, the example incorrectly
instantiates a protocol and passes an undefined build_protocol to
loop.create_connection; remove the unused protocol = protocol_factory() or
replace it by assigning build_protocol = protocol_factory, or simply pass the
callable directly by changing the create_connection call to use protocol_factory
(i.e., ensure loop.create_connection receives the protocol factory callable
rather than a protocol instance), updating the code around protocol_factory,
protocol and build_protocol to be consistent.
In `@src/netius/test/base/container.py`:
- Around line 262-283: In test_mixed_base_and_agent, avoid double-cleanup by
removing the explicit per-agent cleanup calls (server.cleanup(),
http_client.cleanup(), raw_client.cleanup()) — rely on the container lifecycle
and its cleanup via self.container.cleanup()/tearDown; alternatively, if you
must clean them individually, call self.container.remove_base(...) for each (or
ensure agent.cleanup() is idempotent) so the container's cleanup() (which
iterates self.bases) won't invoke cleanup twice.
🧹 Nitpick comments (1)
src/netius/test/base/container.py (1)
149-149: PreferassertIsInstanceoverassertEqual(isinstance(…), True).
assertIsInstanceproduces a clearer failure message ("X is not an instance of dict" vs "False != True").♻️ Proposed fix
- self.assertEqual(isinstance(result, dict), True) + self.assertIsInstance(result, dict)
| ## Bi-directional asyncio Compatibility | ||
|
|
||
| A core design principle of Netius is **bi-directional compatibility** with Python's asyncio. Rather | ||
| than being a one-way wrapper, compatibility works in both directions across three layers: |
There was a problem hiding this comment.
Prefer “Bidirectional” (no hyphen) for standard technical usage.
This is a common style in technical documentation and aligns with asyncio’s wording.
✏️ Suggested wording
-## Bi-directional asyncio Compatibility
+## Bidirectional asyncio Compatibility📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Bi-directional asyncio Compatibility | |
| A core design principle of Netius is **bi-directional compatibility** with Python's asyncio. Rather | |
| than being a one-way wrapper, compatibility works in both directions across three layers: | |
| ## Bidirectional asyncio Compatibility | |
| A core design principle of Netius is **bi-directional compatibility** with Python's asyncio. Rather | |
| than being a one-way wrapper, compatibility works in both directions across three layers: |
🤖 Prompt for AI Agents
In `@doc/architecture.md` around lines 7 - 10, Replace the hyphenated phrase
"Bi-directional asyncio Compatibility" (and any occurrences of "bi-directional
compatibility") with the unhyphenated "Bidirectional asyncio Compatibility"
everywhere in the document; update the section header "Bi-directional asyncio
Compatibility" and any inline uses of "bi-directional compatibility" to
"Bidirectional" to match standard technical usage and asyncio wording.
| 1. Connections are owned by the right Base (eg ProxyServer) | ||
| 2. `poll_owner()` routes their events to that Base's `on_read()` -> `on_data_base()` | ||
| 3. Data reaches the transport -> protocol layer |
There was a problem hiding this comment.
Use “e.g.” instead of “eg” in formal docs.
Minor readability/consistency improvement.
✏️ Suggested wording
-1. Connections are owned by the right Base (eg ProxyServer)
+1. Connections are owned by the right Base (e.g., ProxyServer)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 1. Connections are owned by the right Base (eg ProxyServer) | |
| 2. `poll_owner()` routes their events to that Base's `on_read()` -> `on_data_base()` | |
| 3. Data reaches the transport -> protocol layer | |
| 1. Connections are owned by the right Base (e.g., ProxyServer) | |
| 2. `poll_owner()` routes their events to that Base's `on_read()` -> `on_data_base()` | |
| 3. Data reaches the transport -> protocol layer |
🤖 Prompt for AI Agents
In `@doc/architecture.md` around lines 129 - 131, Replace the informal "eg" with
the standard abbreviation "e.g." in the sentence "Connections are owned by the
right Base (eg ProxyServer)" so the line reads "Connections are owned by the
right Base (e.g. ProxyServer)"; update any other occurrences of "eg" in the same
document for consistency and run a quick spell/grammar check on
doc/architecture.md to ensure punctuation spacing matches the style.
| def _connect_stream_compat(..., loop=None): | ||
| loop = loop or common.get_loop() | ||
| protocol = protocol_factory() | ||
| connect = loop.create_connection(build_protocol, host=host, port=port, ssl=ssl) | ||
| future = loop.create_task(connect) | ||
| future.add_done_callback(on_connect) | ||
| ``` |
There was a problem hiding this comment.
Clarify protocol factory variable naming.
The code example has inconsistent variable naming that may confuse readers:
- Line 71 creates
protocol = protocol_factory()but this variable is never used - Line 72 uses
build_protocol(undefined) as the protocol factory argument tocreate_connection() - This differs from the Native Path example (line 56) which consistently uses
protocol_factory()
Since create_connection() expects a protocol factory callable (not an instance), consider either:
- Removing line 71 and defining
build_protocolas an alias:build_protocol = protocol_factory - Or using
protocol_factorydirectly:connect = loop.create_connection(protocol_factory, host=host, port=port, ssl=ssl)
📝 Suggested fix for consistency
def _connect_stream_compat(..., loop=None):
loop = loop or common.get_loop()
- protocol = protocol_factory()
- connect = loop.create_connection(build_protocol, host=host, port=port, ssl=ssl)
+ connect = loop.create_connection(protocol_factory, host=host, port=port, ssl=ssl)
future = loop.create_task(connect)
future.add_done_callback(on_connect)🤖 Prompt for AI Agents
In `@doc/compat.md` around lines 69 - 75, In _connect_stream_compat, the example
incorrectly instantiates a protocol and passes an undefined build_protocol to
loop.create_connection; remove the unused protocol = protocol_factory() or
replace it by assigning build_protocol = protocol_factory, or simply pass the
callable directly by changing the create_connection call to use protocol_factory
(i.e., ensure loop.create_connection receives the protocol factory callable
rather than a protocol instance), updating the code around protocol_factory,
protocol and build_protocol to be consistent.
| def test_mixed_base_and_agent(self): | ||
| server = netius.StreamServer() | ||
| http_client = netius.clients.HTTPClient() | ||
| raw_client = netius.clients.RawClient() | ||
|
|
||
| self.container.add_base(server) | ||
| self.container.add_base(http_client) | ||
| self.container.add_base(raw_client) | ||
|
|
||
| self.assertEqual(len(self.container.bases), 3) | ||
|
|
||
| # runs all lifecycle operations to ensure no errors | ||
| self.container.start_all() | ||
| self.container.ticks() | ||
| self.container.call_all("on_start") | ||
| self.container.call_all("on_stop") | ||
| self.container.connections_dict() | ||
| self.container.connection_dict(1) | ||
|
|
||
| server.cleanup() | ||
| http_client.cleanup() | ||
| raw_client.cleanup() |
There was a problem hiding this comment.
Same double-cleanup concern applies here — individual cleanup() calls followed by tearDown.
Lines 281-283 explicitly clean up server, http_client, and raw_client. Then tearDown calls self.container.cleanup(), which iterates self.bases (still containing all three) and calls .cleanup() on each again. If the agents' cleanup() is not idempotent this will fail. This pattern recurs in several tests in this file (e.g., test_add_base_agent, test_start_base_agent, test_ticks_agent, etc.).
🤖 Prompt for AI Agents
In `@src/netius/test/base/container.py` around lines 262 - 283, In
test_mixed_base_and_agent, avoid double-cleanup by removing the explicit
per-agent cleanup calls (server.cleanup(), http_client.cleanup(),
raw_client.cleanup()) — rely on the container lifecycle and its cleanup via
self.container.cleanup()/tearDown; alternatively, if you must clean them
individually, call self.container.remove_base(...) for each (or ensure
agent.cleanup() is idempotent) so the container's cleanup() (which iterates
self.bases) won't invoke cleanup twice.
Enables the Container to multiplex both old Base-derived and new Agent/Protocol-derived objects on a shared poll, fixing AttributeError when running ConsulProxyServer. Adds _container_loop bridging, event relay, proxy throttle fixes, and comprehensive test coverage for container setup and proxy data flow.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests