Fix: reject a non-object Reasoning Engine raw body instead of crashing the dev server

[AmaadMartin]

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):
   Closes: #issue_number
   Related: #issue_number

No public issue is associated with this report, so no issue number is cited.

2. Or, if no issue exists, describe the change:

Problem: POST /api/reasoning_engine can be killed by a four-byte request body.

The handler in dev/src/server/adk_api_server.ts has two paths. When express.json()
declines a request (for example the doubled Content-Type: application/json,application/json
that Agent Engine sends, which the existing test at dev/test/server/adk_api_server_test.ts:1125
already exercises), the handler reads the socket itself and parses the body by hand:

let body: any = {};
if (rawBody) {
  try {
    body = JSON.parse(rawBody);
  } catch (e) { ... }
}
await executeQuery(body);

JSON.parse was guarded against invalid JSON but not against valid non-object JSON.
JSON.parse('null') returns null, and the first statement of executeQuery is
const input = body.input || {};, so the handler dereferences null. Three things follow
from that one line:

1. TypeError: Cannot read properties of null (reading 'input').
2. The throw happens inside the async listener passed to req.on('end', ...). EventEmitter
   discards the promise a listener returns, so nothing catches it — an unhandled rejection,
   which has been fatal since Node 15. The dev server process exits.
3. The throw precedes every res.* call and sits outside executeQuery's own try/catch, so
   no response is ever written and the client blocks until its socket times out.

No authentication, no session and no valid app name are needed, so this is an unauthenticated
remote denial of service against the dev server rather than an error-handling nicety.

Only null is fatal. Reading a property off a number, string or boolean returns undefined,
so 5, "hi", true and [] already produce the correct 400 today; they are normalized by
this change but were never crashes. (The originating report claimed bare numbers and strings
also threw — that is not correct, and no test here asserts it.)

Solution: narrow the parsed value to a non-null object before it reaches executeQuery.

let body: object = {};
if (rawBody) {
  try {
    const parsed: unknown = JSON.parse(rawBody);
    if (typeof parsed === 'object' && parsed !== null) {
      body = parsed;
    }
  } catch (e) {
    this.logger.error(`Failed to parse raw body as JSON: ${e}`);
  }
}

A non-object body now means "no usable payload" and falls through to the pre-existing
validation response: 400 {"error":"appName is required in input"} — the same answer the
endpoint already gives for 5, "hi", true and unparseable input. This is behavioral parity
with adk-python's FastAPI equivalent, which rejects a non-conforming body with a 400 rather
than dying; the two endpoints have different error payload shapes and that difference is out of
scope.

Why it is shaped this way:

• The request body is a trust boundary, so the fix is a guard at the point the value enters the
  handler, not a try/catch bolted around the symptom.
• Both operands are load-bearing. typeof null === 'object', so a guard that checks only
  typeof does not fix the bug — see the second mutation below, which is caught.
• No new error type, status code or message. Introducing a distinct "Invalid request body"
  would change an observable contract for a case that already returns the appName message,
  and is not needed to fix the crash.
• No new log line either. An earlier revision logged the discarded body at warn, but the line
  above already logs Received Reasoning Engine raw body: ${rawBody} unconditionally, so the only
  delta was severity — and the rejection is already observable through the 400. Dropped.
• Typing the local as object makes the existing
  // eslint-disable-next-line @typescript-eslint/no-explicit-any obsolete, so this is a net
  removal of one any and one suppression. No suppression is added anywhere — git diff -U0
  over this branch matches zero @ts-expect-error / @ts-ignore / eslint-disable / as any
  additions.
• executeQuery keeps its existing any parameter. Retyping it means modelling the whole
  Reasoning Engine payload, which is a different change from a crash fix.
• No catch-all was added around executeQuery. With the narrowing in place there is no reachable
  throw left outside executeQuery's own try/catch — the property reads cannot throw on a
  non-null object (a truthy non-object body.input such as {"input": 5} yields undefined on
  further reads), and everything past the appName guard is already inside a try that responds 500. A catch-all would be unreachable, and therefore untestable code.

Deliberately not included: consolidating /run, /run_sse and /api/reasoning_engine into a
shared orchestration helper, and introducing a validated request model for the payload. Both are
defensible refactors, and both rewrite the exact lines this fix touches, which would bury a
five-line crash fix in a large diff.

Collision check (gh pr list --repo AmaadMartin/adk-js --state open --limit 1000, all 379
open PRs scanned, then gh pr diff --name-only on every plausibly adjacent one):

• Fix: narrow the reasoning engine request body instead of typing it any (Part 3/3) #232 — "narrow the reasoning engine request body instead of typing it any (Part 3/3)"
  overlaps functionally. It rewrites the same block to call a new
  parseReasoningEngineQuery() whose first line is isRecord(rawBody) ? rawBody : {}, which also
  removes this crash as a side effect. I extracted that module from the PR head and ran it on Node
  v22.22.2: for null, "hello", 5, true and [] it returns appName === undefined without
  throwing, i.e. the same 400. It is not stacked on here for two reasons: it is part 3 of an
  unmerged three-PR stack (Fix: type the searchFlights result without any (Part 1/3) #230 → Fix: omit conformance event fields by destructuring instead of any (Part 2/3) #231 → Fix: narrow the reasoning engine request body instead of typing it any (Part 3/3) #232, each based on the previous rather than on main),
  so the crash fix only reaches main if all three land; and it expands scope with a new module
  and a new mandatory-newMessage 400. This PR is the standalone minimum that can land on its own.
  If Fix: narrow the reasoning engine request body instead of typing it any (Part 3/3) #232's stack lands first, this PR should be closed as redundant.
• Fix: decode the Reasoning Engine raw body with setEncoding so multi-byte UTF-8 survives chunk boundaries #472 — "decode the Reasoning Engine raw body with setEncoding" touches the adjacent
  req.on('data', ...) lines in the same block but leaves the null dereference intact. Adjacent,
  not overlapping; the two diffs do not touch a common line.
• Refactor: Extract a shared Vertex AI reasoning engine resource-name parser #227 (Vertex AI reasoning engine resource-name parser) and Fix: force established sockets shut in AdkApiServer.stop() so teardown always settles #452 (AdkApiServer.stop()
  socket teardown) are unrelated despite matching on name.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.

Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

Two new cases in the existing describe('Reasoning Engine', ...) block of
dev/test/server/adk_api_server_test.ts. No existing test was modified, removed, skipped or
weakened — the five original Reasoning Engine tests are the regression signal that behavior for
well-formed bodies is unchanged, and all five still pass untouched.

Both use raw fetch rather than the file's HttpClient helper, which cannot express these
requests: it hardcodes Content-Type: application/json (routing to the already-parsed path) and
its body ? JSON.stringify(body) : undefined ternary drops a falsy payload, so it physically
cannot send the literal null.

1. should return 400 when the raw body is JSON null — the crash regression test.
2. should return 400 when the raw body is a JSON primitive ("hello") — required for branch
   coverage: without it the typeof parsed === 'object' operand never evaluates to false.

Together with the pre-existing raw-body test (a well-formed object), all three states of the new
branch are exercised: typeof true + !== null true → assign; typeof true + !== null false →
skip; typeof false → skip.

npx vitest run --project unit:dev dev/test/server/adk_api_server_test.ts
  ✓ |unit:dev| dev/test/server/adk_api_server_test.ts (53 tests) 556ms
  Test Files  1 passed (1)
       Tests  53 passed (53)

Proof the tests can fail (mutation testing). Both mutations were run one at a time, with the
source restored from a pristine copy in between.

Mutation 1 — revert the fix hunk to the pre-fix body = JSON.parse(rawBody);. Test 1 fails
loudly:

⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯
TypeError: Cannot read properties of null (reading 'input')
 ❯ executeQuery dev/src/server/adk_api_server.ts:811:28
 ❯ IncomingMessage.<anonymous> dev/src/server/adk_api_server.ts:875:17
 ❯ IncomingMessage.emit node:events:519:28
Error: Hook timed out in 10000ms.
 Test Files  1 failed (1)
      Tests  1 failed | 52 skipped (53)

The fetch never resolves (no response is written), so the teardown hook times out as well —
the same two symptoms a real client sees.

Mutation 2 — keep the guard but drop the null operand (if (typeof parsed === 'object')),
which is the most plausible way to get this fix wrong since typeof null === 'object'. Test 1
fails identically:

⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯
TypeError: Cannot read properties of null (reading 'input')
 Test Files  1 failed (1)
      Tests  1 failed | 52 skipped (53)

Honest note on test 2: it is a coverage test, not a regression test. "hello" does not throw
on the unfixed code — it reaches the 400 by accident — so test 2 still passes under both
mutations. It is included because dropping it leaves the typeof operand's false arm unexecuted,
and it pins the 400 contract for primitives against future refactors of this block.

Coverage. Scoped run over the changed file:

npx vitest run --project unit:dev dev/test/server/adk_api_server_test.ts \
  --coverage.enabled --coverage.include='dev/src/server/adk_api_server.ts'

Every line and branch added by this change is covered. The only uncovered lines inside the block
are 874-875, the pre-existing JSON.parse syntax-error catch arm, which this diff does not
touch and which was equally uncovered before the change.

Repo gates run locally on the pushed commit:

• npm run build — pass.
• npm run lint — pass, clean.
• npm run format:check — "All matched files use Prettier code style!".
• npm run ts:check — 281 errors, all pre-existing in core/test, zero in dev/ and zero
  delta introduced by this branch (281 on the base commit, 281 with the change; verified by
  stashing). ts:check is not a step in .github/workflows/validation.yaml.

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

1. npm run build at the repo root.

2. Start the dev server against any agents directory (adk api_server <agents_dir>).

3. Send the four-byte payload that used to kill it:

   curl -i -X POST http://localhost:<port>/api/reasoning_engine \
     -H 'Content-Type: application/json,application/json' \
     --data 'null'

   Expect HTTP/1.1 400 Bad Request and {"error":"appName is required in input"}. Before this
   change the request returns nothing at all and the server process exits.

4. Immediately re-issue a valid Reasoning Engine query and confirm it returns 200. This is the
   half of the bug the status code alone does not demonstrate: the process survived.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.

CI note. The first run-tests (windows-latest) attempt failed on three unrelated timeouts —
tests/e2e/tools/mcp/load_mcp_resource_e2e_test.ts, two cases in
tests/integration/app_loader/app_loader_test.ts, and tests/integration/build_setup/build_setup_test.ts.
None of them exercise the dev HTTP server, and the unmodified base commit b390217e fails on
windows in the same way (on tests/integration/adk_web/webui_test.ts). Re-running the job passed:
run-tests, ubuntu-latest, macos-latest and windows-latest are all green on this commit.

Revision after complexity review. The else { this.logger.warn(...) } arm was removed: line
865 already logs the raw body unconditionally, so the warn added only severity, and the discarded
body is observable through the 400. The guard is now four lines shorter. Both mutations were
re-run against the shrunk code and still fail (Unhandled Rejection TypeError: Cannot read properties of null (reading 'input') + Hook timed out in 10000ms), all 53 tests still pass, and
the new lines remain fully covered. The reviewer's optional braceless-if collapse was not taken:
every if in this 1060-line file uses braces, and no curly rule is configured, so the braces
match local style.