Loopback server sockets, so MCP can serve any platform - #5472
Loopback server sockets, so MCP can serve any platform#5472shai-almog wants to merge 24 commits into
Conversation
MCP's socket transport was JavaSE-only, and for a real reason: it binds a server socket, and the portable com.codename1.io.Socket API only binds the wildcard address — which would publish a channel that can read the screen and drive the UI on every network interface. So attaching an agent to an app running on a device was simply not possible. This adds the missing capability rather than working around it: - Socket.listenLoopback(port, class) and isLoopbackServerSocketSupported(), backed by a new CodenameOneImplementation.listenSocketLoopback hook. It is deliberately SEPARATE from listenSocket and defaults to unsupported: a port must implement loopback binding explicitly, because answering this with the wildcard implementation would silently widen the reach of a channel the caller asked to keep local. There is no fallback for the same reason. - MCPLoopbackSocketTransport in core, built on that API, used automatically by any port that did not register a transport of its own. JavaSE keeps its java.net implementation. - JavaSE and Android bind InetAddress.getLoopbackAddress(). Their server-socket caches key loopback and wildcard sockets separately, so a port already bound wildcard can never be handed to a caller that asked for loopback. - iOS gains server sockets at all: it had none (listenSocket returned null). The native implementation binds INADDR_LOOPBACK, listens, and wraps each accepted descriptor in a CFStream pair, yielding the VM thread across the blocking accept. Wildcard listening stays unsupported there, so an iOS app cannot accidentally publish a port. Tests cover the framing and session handling: frame delimiting independent of packet boundaries, CRLF tolerance, write-with-no-client, reconnect without ending the server, and close waking a pending read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The loopback listener accepted connections but no data ever crossed them. The accepted descriptor was wrapped in a CFStream pair scheduled on [NSRunLoop currentRunLoop] — which is what the client-side connect path does, and correct there — but a server socket is read on a background thread whose run loop never runs, so the streams opened and then delivered nothing. Accepted sockets now use recv/send on the descriptor directly, with FIONREAD for available input and a blocking read (the caller is a reader thread that expects to wait, and a non-blocking nil return would spin). The client path is untouched. Verified end to end on the iOS simulator: an agent attaches to a running app, reads its accessibility tree, and drives it through several screens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds loopback-only server socket support to Codename One so MCP can offer a safe “attach to a running app” socket transport on non-JavaSE ports (device/simulator) without exposing the channel on all network interfaces.
Changes:
- Adds core loopback server-socket API (
Socket.listenLoopback()/isLoopbackServerSocketSupported()) backed by a newCodenameOneImplementation.listenSocketLoopback()hook. - Implements loopback listening in JavaSE/Android, and introduces loopback-only server sockets on iOS via native
INADDR_LOOPBACKbinding + raw-fd IO for accepted sockets. - Adds a portable
MCPLoopbackSocketTransportfallback (used when a port doesn’t register its own socket transport) and unit tests covering framing/session behavior.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Ports/JavaSE/src/com/codename1/impl/javase/JavaSEPort.java | Adds loopback-binding support and separates cached wildcard vs loopback server sockets. |
| Ports/Android/src/com/codename1/impl/android/AndroidImplementation.java | Adds loopback-binding support and separate server-socket caching for wildcard vs loopback. |
| Ports/iOSPort/src/com/codename1/impl/ios/IOSNative.java | Declares native loopback listen entrypoint for iOS. |
| Ports/iOSPort/src/com/codename1/impl/ios/IOSImplementation.java | Exposes loopback server socket support via listenSocketLoopback() and capability flag. |
| Ports/iOSPort/nativeSources/SocketImpl.m | Implements loopback-only listening on iOS and raw-fd read/write for accepted sockets. |
| Ports/iOSPort/nativeSources/SocketImpl.h | Adds raw-fd field and loopback listen method declaration for server-side sockets. |
| Ports/iOSPort/nativeSources/IOSNative.m | Bridges iOS native loopback listen into the Java layer. |
| CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java | New portable MCP loopback socket transport built on com.codename1.io.Socket. |
| CodenameOne/src/com/codename1/mcp/MCP.java | Registers portable socket transport fallback and updates socket support detection. |
| CodenameOne/src/com/codename1/io/Socket.java | Adds loopback listen API and routes listening through loopback vs wildcard accept hooks. |
| CodenameOne/src/com/codename1/impl/CodenameOneImplementation.java | Introduces loopback server socket capability hook with safe default (unsupported). |
| maven/core-unittests/src/test/java/com/codename1/testing/TestCodenameOneImplementation.java | Extends test implementation to support loopback server sockets for unit tests. |
| maven/core-unittests/src/test/java/com/codename1/mcp/MCPLoopbackSocketTransportTest.java | Adds tests for message framing and reconnect/close semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Compared 12 screenshots: 12 matched. |
Cloudflare Preview
|
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Four review findings plus the static-analysis and header gates. - The server-socket cache distinguished loopback from wildcard by negating the port. -0 == 0, so on an ephemeral-port listener (port 0) a loopback request could be handed the wildcard-bound socket, defeating the one guarantee this API exists to make. Loopback and wildcard sockets now live in separate maps, so no encoding can collide. Fixed in both the JavaSE and Android ports. - Socket.listenLoopback() documented that it fails when the platform has no loopback support, but it started a listener thread that hit the default implementation's exception and merely logged it — the caller believed it was listening. It now throws IllegalStateException on the calling thread. - The iOS listenSocketLoopback binding released nothing when the bind or accept failed, leaking a SocketImpl per retry. (The adjacent connectSocket has the same pre-existing shape; left alone to keep this change reviewable.) - PMD: the identity comparisons and borrowed-stream locals in the transport are deliberate — a singleton registration slot and streams owned by the connection callback — so they carry NOPMD markers explaining why, rather than relaxing CompareObjectsWithEquals or CloseResource for the whole codebase. - SocketImpl.h/.m never carried a copyright header; modifying them tripped the header gate. Added the standard header. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
Ports/iOSPort/nativeSources/SocketImpl.m:200
- In the raw-fd server-side path,
writeToStream:marks the socket disconnected onsend()failure but does not closerawFd. This can leak the accepted file descriptor on client disconnects or write errors, and may leaverawFdpointing at an unusable socket until an explicit disconnect happens.
if(rawFd >= 0) {
const uint8_t* bytes = (const uint8_t*)[param bytes];
size_t remaining = [param length];
while(remaining > 0) {
ssize_t w = send(rawFd, bytes, remaining, 0);
if(w <= 0) {
connected = NO;
return;
}
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
The Android port compiles CodenameOne/src through ant javac with ASCII
encoding, so the em dashes in the doc comments I added were hard errors
("unmappable character for encoding ASCII"), not warnings. Replaced with
hyphens; master carried no non-ASCII in these files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Serving MCP over a socket used to be JavaSE only, which meant simulator and desktop tooling: inherently a development environment. Now that the transport works wherever loopback can be bound, the same call can bind inside an app on a phone, and the risk profile is not the same. An attached agent reads the screen and drives the UI, and loopback is shared by everything on the device rather than being private to one app. So on a shipped build, any OTHER installed app could connect to the port and drive yours. Nothing binds unless the app calls a starter, and an unused MCP is stripped by the VM, so this cannot happen by accident -- but a starter left in the code is exactly how a debugging facility reaches production. startSocketServer now refuses on a release build. A development build is: - Android: the package carries FLAG_DEBUGGABLE, which the build sets for a debug variant and clears for a release variant. - iOS: the provisioning profile grants get-task-allow, the entitlement that lets a debugger attach. Development and ad-hoc profiles carry it, App Store and enterprise profiles do not. Unlike the Xcode DEBUG macro this does not depend on which configuration the build server compiled. - JavaSE: always, since that port is the simulator, the designer and the desktop tooling, and its MCP menu already serves agents from there. The default for a port that cannot tell is "release", so the answer errs towards withholding the facility rather than exposing it. setAllowOnReleaseBuilds(true) lifts the block for a build that ships to a controlled fleet. The underlying question is more broadly useful than MCP, so it is exposed as Display.isDebuggableBuild() and documented for gating any development-only facility. Verified in the built artifacts of all three ports, not just at the source level; the iOS native function was clang syntax checked for device and simulator targets. Guide chapter updated with a "Development builds only" section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
CodenameOne/src/com/codename1/mcp/MCP.java:132
- The startSocketServer() JavaDoc still says it requires a port-registered socket transport factory, but the implementation now falls back to the portable loopback transport when available. Updating this line will prevent API consumers from concluding socket attach is JavaSE-only.
/// Requires a platform socket transport factory (registered by the JavaSE port).
Ports/iOSPort/nativeSources/SocketImpl.m:179
- In the rawFd (accepted-socket) path, recv() returning 0/-1 marks connected=NO but leaves the file descriptor open. Because SocketInputStream/SocketOutputStream only call disconnectSocket() when isSocketConnected() is true, this can leak accepted client fds on orderly disconnects and read errors. Close rawFd when recv() indicates EOF/error.
if(r > 0) {
return [NSData dataWithBytes:buffer length:r];
}
connected = NO;
return nil;
Ports/iOSPort/nativeSources/SocketImpl.m:304
- Accepted sockets use raw send()/recv(). On iOS/macOS, a write to a closed socket can raise SIGPIPE unless suppressed, which in this codebase is routed through a signal handler and can surface as an unexpected RuntimeException. Consider disabling SIGPIPE for the accepted clientFd (e.g., SO_NOSIGPIPE) so send() reports EPIPE instead of generating a signal.
// Serve this socket with raw descriptor I/O. An accepted connection is read on a
// background thread whose run loop never runs, so a scheduled CFStream would open
// but never deliver a byte.
rawFd = clientFd;
connected = YES;
|
Compared 217 screenshots: 217 matched. |
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
|
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
|
Compared 144 screenshots: 144 matched. |
|
Compared 181 screenshots: 181 matched. |
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Seven review findings, all real. Register the portable transport only when loopback can actually be bound. registerIfPlatformHasNone installed it regardless, so socketTransportFactory became non-null on a platform that cannot bind, startSocketServer reported success, and the open failed later on the server's own thread where no caller could see it. The synchronous "no socket transport" refusal the message promises now actually happens. This is the same defect my own gate test tripped over -- it had to assert the wrong thing to pass. Discard a connection accepted after stop(). The listener re-checks the flag after accept returns, because stop() can be called while the thread is parked inside accept, and the connection that unblocks it belongs to a listener the caller has already abandoned. Previously that last connection was served. Close the accepted descriptor when the peer goes away. recv returning 0 or an error, and a failed send, marked the socket disconnected but left the fd open; the Java side only calls disconnectSocket while the socket still reports itself connected, so the descriptor was stranded for the life of the process. Both paths now go through one closeRawDescriptor. Suppress SIGPIPE on accepted sockets. This process installs a SIGPIPE handler (CodenameOne_GLAppDelegate), so a write to a departed peer -- the ordinary way a client disconnects mid-response -- would surface as a signal rather than the EPIPE the write path is written to handle. SO_NOSIGPIPE on the accepted fd makes it a plain error. Yield across send. accept and recv already hand the thread back to the VM; send blocks once the send buffer fills, and holding the thread there stalled cooperative scheduling for as long as the peer was slow to drain. Also drop an adverb the developer-guide Vale gate rejected. The iOS changes were clang syntax checked against the device and simulator SDKs and confirmed present in the packaged nativeios bundle, not just in the working tree. 32 MCP tests pass, including a new one covering the synchronous refusal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
End of stream with no newline returned the partial buffer as if it were a message. For a newline-delimited protocol that is a truncated frame, which means the client went away mid-message -- so the server would parse a broken request, fail to answer it (the peer is already gone), and end its loop. That defeats the one thing this transport is built to do: stay bound so the next agent session can attach. readLine now reports end of stream regardless of how many bytes arrived first, which is the same path a clean disconnect already took. The new test was control-tested against the previous form to confirm it fails there rather than passing for unrelated reasons. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
close() forgot the output stream rather than closing it. A writer that had already captured it would go on writing into a session that had ended, and the underlying socket stayed open until the connection callback happened to unwind. Both streams now come out of their fields under the lock and are closed outside it, through the same closeQuietly helpers attach() uses. New test covers it, control-tested against the previous form. The guide anchor was [[Development builds only]] -- an anchor ID cannot contain spaces, so the cross-reference to it was malformed even though asciidoctor did not fail the build. Now [[development-builds-only]], referenced with explicit link text so the rendered wording is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
isDebuggableBuild() re-read and re-parsed embedded.mobileprovision on every call. The profile is fixed for the life of the process, and this is consulted by the MCP gate and exposed publicly through Display.isDebuggableBuild(), so a caller is free to ask repeatedly -- file IO plus a plist parse each time is a poor answer to that. It is now computed once under dispatch_once. Verified by re-running the extracted-function harness: the four fixtures still classify identically (development, distribution-with-beta-reports-active, corrupt, missing), and a second harness with the once-token left intact shows the second call returning the first answer without re-reading, which is the cache doing its job. toUtf8 used buffer.toByteArray(), which copies the whole payload before decoding it -- megabytes of pointless duplication for a frame near the ceiling. ByteArrayOutputStream.toString(String) decodes straight out of the internal buffer, and ParparVM implements it as new String(buf, 0, count, charsetName), so the saving is real on device and not only on the JVM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
buffer.toString("UTF-8") broke the build. Core is compiled with
-bootclasspath Ports/CLDC11/dist/CLDC11.jar, and CLDC11's ByteArrayOutputStream
declares only the no-argument toString(). The overload exists on the JDK and in
the ParparVM runtime, which is why a Maven build of core accepts it and the Ant
build then fails -- and why checking the ParparVM sources, as I did, was not a
check of anything core actually compiles against.
Restored the toByteArray() form with a note on the method so the next person is
not tempted by the same avoidable copy.
Also declare string.h explicitly for memset. It resolves today only because
Foundation drags it in, which is not something to rely on.
Verified this time by running the compile CI runs -- ant -buildfile
CodenameOne/build.xml jar against a locally built CLDC11 jar -- rather than
inferring from Maven, which cannot catch this class of error at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closing the listening socket is how a stop is delivered, and it surfaces in the loop as a failed accept. The null-connection branch then reported that through connectionError, announcing a fault that never happened, and passed a null connection to getSocketErrorCode/Message on the way. It now breaks out when a failed accept coincides with a stop, before any callback object is built. The backoff after a genuine accept failure is slept in slices with the flag re-tested between them, so a stop arriving during the pause is acted on within 50ms rather than after the full 500. Tests: assert the exception message is non-null before matching on it, in the two places that were still calling getMessage() straight into contains() -- the same guard the refusal helper already had. And the wait helper's deadline is System.nanoTime() rather than currentTimeMillis, so an NTP step mid-wait cannot cut the wait short or stretch it. Verified with the Ant CLDC11 core compile FIRST this time, then the Maven build, 41 MCP tests and PMD. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…factories
readLine consumed the stream a byte at a time, so a frame near the 8MB ceiling
cost millions of single-byte reads -- cheap to trigger from the other side of a
loopback connection. It now reads in 8KB blocks.
Buffering is only safe if bytes past the delimiter are KEPT rather than swallowed,
which was the reason the byte-at-a-time loop existed. They are held in a
per-transport chunk and consumed by the next call, and the chunk is discarded
whenever the attached stream is not the one that filled it -- otherwise a new
client's first message would be a dead client's tail. A test covers both halves,
and control-testing the second one returns {"id":3} to the new client instead of
its own {"id":9}, which is one session's data delivered to another.
hasPlatformSocketTransport, isStdioSupported and isSocketSupported read the
factory fields without holding the class monitor while everything else in the
class takes it. The reads and the two setters are now synchronized. I tried
volatile first; PMD forbids it here (AvoidUsingVolatile), which also matches the
review preference expressed earlier on this PR.
getStoppedListeners returns an unmodifiable snapshot rather than the live list, so
a test cannot mutate what a later assertion reads.
Verified with the Ant CLDC11 core compile, the Maven build, 42 MCP tests and PMD.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t retries connectionError was an empty method, so a listener that could never bind failed silently: startSocketServer returns, the server reports itself running, and nothing can attach - a port already in use being the ordinary way that happens. It now logs, de-duplicated on the message so a permanent failure says so once rather than once per retry, and falls back to stderr because the log routes through the platform implementation, which may not be registered when a server starts early in initialization. The retry itself was a fixed 500ms, so a port that can never be bound meant two callbacks a second for the life of the process. The backoff now starts at 50ms, doubles while the failure persists and is capped at 5s, and resets as soon as an accept succeeds so a listener that saw one bad moment is not left sluggish. Still slept in slices, so stopping is noticed within 50ms regardless of how long the backoff has grown. Also dropped "which this codebase does not use" from a comment. It was a global claim in a local rationale, and the sort of thing that quietly becomes false. Verified with the Ant CLDC11 core compile first, then Maven, 42 MCP tests, PMD. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
CodenameOne/src/com/codename1/io/Socket.java:264
- The second stop check (
if (connection == null && stopped.get())) is redundant/unreachable because the immediately precedingif (stopped.get()) { ... break; }already handles the stop-delivered-via-accept-returning case (includingconnection == null). Removing the dead branch will reduce confusion and keep the stop semantics in one place.
while (!stopped.get()) {
final Object connection = loopbackOnly
? Util.getImplementation().listenSocketLoopback(port)
: Util.getImplementation().listenSocket(port);
if (stopped.get()) {
// stop() was called while this thread sat inside accept. The
// connection that unblocked it belongs to a listener the caller
// has already abandoned, so hand it back rather than serving it.
if (connection != null) {
Util.getImplementation().disconnectSocket(connection);
}
break;
}
if (connection == null && stopped.get()) {
// Closing the listening socket is how a stop is delivered, and
// it surfaces here as a failed accept. Reporting that through
// connectionError would announce a fault that never happened.
break;
}
CodenameOne/src/com/codename1/mcp/MCP.java:35
- This class-level doc says the socket transport is "blocked on a release build" without noting the documented JavaSE exception (JavaSE always reports debuggable). Consider amending this sentence to match the more precise semantics described later (i.e., blocked when the port can reliably report a release build; JavaSE cannot distinguish packaged desktop apps).
/// The stdio transport is supported by the Codename One JavaSE port, which powers the
/// simulator and the desktop tooling. The socket transport also serves a build running on
/// a device, on any port that can bind the loopback interface. It is blocked on a release
/// build - see [#setAllowOnReleaseBuilds(boolean)].
CodenameOne/src/com/codename1/mcp/MCPLoopbackSocketTransport.java:400
messagemay be null (depending on port error reporting), which would produce logs like... failed: null (code). Consider normalizing null/empty messages to a stable fallback (e.g., "unknown error") before formatting, so the log remains actionable and deduping stays meaningful.
public void connectionError(int errorCode, String message) {
// Without this the failure is silent: startSocketServer returns, the server
// reports itself running, and nothing can ever attach - a port already in use
// being the ordinary way that happens. Logged rather than thrown because this
// arrives on the listener thread, long after the caller has gone.
String description = "MCP socket listener failed: " + message + " (" + errorCode + ")";
if (description.equals(lastReportedError)) {
return; // the same failure retrying; saying so once is enough
}
Loopback server sockets, so MCP can serve any platform
MCP's socket transport was JavaSE-only, and for a real reason: it binds a server
socket, and the portable
com.codename1.io.SocketAPI only binds the wildcardaddress — which would publish a channel that can read the screen and drive the UI
on every network interface. Attaching an agent to an app running on a device
was therefore not possible at all.
This adds the missing capability rather than working around it.
Core API
Socket.listenLoopback(port, class)andSocket.isLoopbackServerSocketSupported(),backed by a new
CodenameOneImplementation.listenSocketLoopbackhook.listenSocketand defaults tounsupported. A port has to implement loopback binding explicitly, because
answering this call with the wildcard implementation would silently widen the
reach of a channel the caller asked to keep local. There is no fallback, for
the same reason.
MCPLoopbackSocketTransportlives in core, built on that API, and is usedautomatically by any port that did not register a transport of its own. JavaSE
keeps its existing
java.netimplementation.Ports
JavaSE / Android bind
InetAddress.getLoopbackAddress(). Their server-socketcaches key loopback and wildcard sockets separately, so a port already bound to
the wildcard address can never be handed to a caller that asked for loopback.
iOS gains server sockets at all — it previously had none (
listenSocketreturned
null). The native implementation bindsINADDR_LOOPBACK, listens, andyields the VM thread across the blocking
accept. Wildcard listening staysunsupported there, so an iOS app cannot accidentally publish a port.
Accepted sockets use
recv/sendon the raw descriptor, withFIONREADforavailable input and a blocking read. The obvious implementation — wrapping the
accepted descriptor in a
CFStreampair, which is what the client-side connectpath does and is correct there — accepts connections but never delivers data: a
server socket is read on a background thread whose run loop never runs. The
client path is untouched.
Tests
MCPLoopbackSocketTransportTestcovers framing and session handling: framedelimiting independent of packet boundaries, CRLF tolerance, write with no client
attached, reconnect without ending the server, and close waking a pending read.
Verified end to end on the iOS simulator: an agent attaches to a running app,
reads its accessibility tree, and drives it through several screens.
No existing public API changed.
🤖 Generated with Claude Code