Repository navigation
refactor(proxy): export the Node adapter from the main entry point - #3262
Conversation
createDaemonProxyServer and createDaemonProxyRequestListener move from @agent-device/proxy/node into @agent-device/proxy, so the package has one entry point. The subpath has not been released yet. The Node adapter now answers CONNECT, TRACE and TRACK with 404, as the proxy did before the Fetch rewrite. Before this change it dropped the socket, because Fetch refuses to construct a Request with those methods.
The package has one entry point now, so the exports map and the fallow entry list name only src/index.ts.
Size Report
Startup median (7 runs, lower is better):
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 15 files
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
…ith 400 Node never hands CONNECT or TRACK to the request listener, so the catch-all 404 only reached TRACE, and it misreported credentialed URLs, which Fetch also refuses, as 404. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Exporting the adapter from the main entry made it evaluate one more module on import, which the eager-closure gate refuses. The server and listener now live beside createDaemonProxy and import node-http.ts on the first request, as health helpers already do. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
Loading node-http.ts on the first request lets the response close before the disconnect listeners attach, so the signal also checks res.closed. Adapter tests assert the response ends again. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
This PR is ready at b8c0ef3. The Node adapter export looks right, and the checks show 19 passing and 0 failing. There are no conflicts. It only needs a maintainer approval to merge. Not blocking: the TRACE, credential, and client-gone tests in packages/proxy/src/node-http.test.ts call serveProxyRequest directly, so no package-local test covers the lazy-import wrapper in createDaemonProxyRequestListener or its destroy-on-error catch (a small step would be to send the TRACE test through that listener), and daemon-proxy.ts, described as the transport-neutral core, now imports node:http eagerly and owns the Node server factory (the factory could live in index.ts or a small node-server.ts instead). Take or leave both. All four cubic-dev-ai threads (P2/P3) are fixed at this commit, so please resolve them: the 404 and credentialed-URL handling (#3262 (comment)), the TRACE-only 404 branch (#3262 (comment)), the ended:true assertions in the 400 tests (#3262 (comment)), and the res.closed check for client disconnects (#3262 (comment)). I did not re-run the eager-closure budget test or a packed install, so the module count and exports-map claims rest on reading the code and on green CI. I checked res.closed on Node v26 only. engines allows >=22.12, and I did not confirm OutgoingMessage.closed on Node 22, but if it is missing there, behavior falls back to the earlier listeners and does not regress. I also did not test TRACE and CONNECT routing in node:http here. |
thymikee
added
the
ready-for-human
|
1 participant
Summary
Follow-up to #3256.
@agent-device/proxynow has one entry point. The Node adapter moves from@agent-device/proxy/nodeinto the main entry. The subpath was never published, so nothing outside this repo depends on it.Importing the package still evaluates the same 8 modules as before.
createDaemonProxyServerandcreateDaemonProxyRequestListenerlive indaemon-proxy.ts, and the Node request adapter innode-http.tsloads on the first request, as the health helpers already do.The Node adapter now answers TRACE with 404, as the proxy did before the Fetch rewrite. Before this change it dropped the socket, because Fetch refuses to construct a
Requestwith that method. Review of #3256 found this. TRACK and CONNECT never reach the listener:node:httpanswers TRACK with 400 and routes CONNECT to its'connect'event, which drops the socket, as it did before the rewrite. A URL with credentials, which Fetch also refuses, now gets 400.15 files: the package exports and build entry, the README, the CLI and test imports, and a gates commit for the exports map and fallow entries.
Validation
Tested commit
5ed888a3f:vitest run scripts/__tests__/eager-closure-budgets.test.tspassed. This is the check that failed on the Coverage job.pnpm check:affected --runpassed, including lint, typecheck, layering, fallow and build.vitest run packages/proxy src/__tests__/daemon-proxy.test.ts: 25 passed. A new test sends TRACE to a realhttp.createServerand gets 404. Another checks that a Host with credentials gets 400.exportslists only.and./package.json.dist/index.mjsexportscreateDaemonProxy,createDaemonProxyServerandcreateDaemonProxyRequestListener.No device-facing change.