CNC-1364: revert and fix keep alive agent

[johnbastian-trayio]

Revert the change from #107 and fix keep alive properly

[Copilot]

🟡 Changes recommended

The PR adds checked-in TLS private key/cert fixtures (including date-sensitive validity), which can trigger security/scanning issues and make tests fail depending on system time.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR reworks ThreadNeedle’s approach to mitigating AWS NAT Gateway idle timeouts by attaching a per-request TCP keep-alive agent (rather than relying on Node/needle HTTP keep-alive pooling behavior), and adds tests to validate protocol correctness and in-flight socket keep-alive behavior.

Changes:

• Introduces lib/addMethod/keepAliveAgent.js to set SO_KEEPALIVE on newly created sockets (HTTP+HTTPS) while avoiding connection pooling and TLS option leakage.
• Updates addMethodREST to attach the agent per request (with opt-out / caller override behavior) instead of setting a global needle default agent.
• Adds unit + integration tests for protocol selection, proxy behavior, keep-alive arming timing, and agent override behavior.

File summaries

File	Description
lib/addMethod/keepAliveAgent.js	New per-request agent resolver that sets TCP keep-alive on socket creation and selects the correct protocol agent.
lib/addMethod/addMethodREST.js	Removes global default agent and attaches a resolved agent to each request’s options.
tests/keepAliveAgent_test.js	Unit tests covering protocol selection, caller overrides, proxy bypass, and agent isolation.
tests/keepAlive_integration_test.js	Integration tests asserting HTTPS works and keep-alive is armed before the response arrives, plus override/opt-out cases.
tests/fixtures/localhost-key.pem	Adds a TLS private key fixture for local HTTPS integration tests.
tests/fixtures/localhost-cert.pem	Adds a TLS certificate fixture for local HTTPS integration tests.
tests/addMethodREST_test.js	Fixes server shutdown to use server.close(done) for proper async completion.
.eslintrc.js	Enables no-new-require to prevent invalid new require('x') usage.
package.json	Bumps package version to 1.19.1.
package-lock.json	Updates lockfile version fields to 1.19.1.

Review details

• Files reviewed: 9/10 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[andrei-tray]

Doesn't this break redirects that switch protocol? resolveAgent picks the agent from the initial URL, but needle reuses it when following redirects — so with follow_max set, an http:// call redirecting to https:// throws an uncaught ERR_INVALID_PROTOCOL and crashes the process (reproduced locally).

[Copilot]

🟢 Approval recommended

The behavioral change is well-contained and covered by new unit/integration tests, with only a minor misleading test comment to optionally correct.

Review details

• Files reviewed: 7/8 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It changes the core HTTP request path and socket/agent behavior for all outbound calls, which can have subtle runtime/network interactions best validated with a final human review.

Review details

• Files reviewed: 10/11 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It changes core HTTP socket/agent behavior across all REST requests and should be manually validated in a real deployment environment for network/proxy/TLS edge cases.

Review details

• Files reviewed: 10/11 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It changes low-level HTTP/TLS socket behavior (agent/protocol/keep-alive) with potentially broad runtime impact that should be validated by a human reviewer in addition to the new tests.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

lib/addMethod/globalize/disableKeepAliveAgent.js:4

• Doc comment is ambiguous: “Whether to skip the keep-alive agent. On by default” reads like the skip-flag is enabled by default, but the implementation defaults to returning false (agent enabled). Reword to clearly state that the keep-alive agent is enabled by default and disableKeepAliveAgent opts out.

• Files reviewed: 10/11 changed files
• Comments generated: 0 new
• Review effort level: Lite