[jochen-testingbot]

Follow-ups from the scan of main requested after #4. Each item is independent.

Stacked on #4. Base is fix/w3c-capabilities-and-integration-coverage, not main, because item 5 touches the integration tests that PR introduces. GitHub will retarget this to main automatically once #4 merges.

1. Credential leak into the appium-mcp child

filterEnv stripped TESTINGBOT_KEY, TESTINGBOT_SECRET, TB_KEY, TB_SECRET — but config.ts also accepts TESTINGBOT_USERNAME and TESTINGBOT_ACCESS_KEY as fallbacks. Anyone configured via those names handed their credentials straight to the child process, contradicting the documented contract that the child gets credentials only baked into the injected hub URL.

The list is now an exported CREDENTIAL_ENV_NAMES, with two tests: one asserting nothing leaks into the spawned env (while unrelated vars still pass through), and one that reads config.ts and fails if getConfig() grows a fallback the filter doesn't cover.

2. tb_appiumEndpoint never existed

It was named in tb_listSessions' agent-facing description, so the agent was being told to call a tool that isn't registered.

What it should use: appium_session_management with action: 'list' — that's the proxied tool that actually owns mobile sessions. The description now says so explicitly, and states that mobile sessions are not listed by tb_listSessions.

Added tests/tools/tool-references.test.ts, which walks every registered description and fails on any tb_* reference that isn't a registered tool. This class of bug can't recur silently.

3. Stale comment in register.ts

Claimed a fallback tool is registered when the appium spawn fails; the catch block only logs. Comment now matches the code. (CLAUDE.md's version of this text was already accurate.)

4. Node floor

preflight() enforced 18; package.json declares >=20. Now 20, with a test at 18.20.0 to keep the runtime gate and engines.node from drifting apart.

5. waitUntil — implemented, not dropped

It was being passed by the integration tests and silently ignored, since tb_navigate's schema only accepted sessionId and url.

Implemented, because the underlying problem is real: navigateTo returns per the session's pageLoadStrategy, which on a page with slow third-party resources can leave the agent looking at a half-built DOM. tb_navigate now confirms document.readyState itself:

• load (default) — complete
• domcontentloaded — interactive or better, returns earlier on ad-heavy pages
• none — return as soon as the driver's call resolves

A timeoutMs (default 30s) bounds the poll. Timeout warns rather than failing the call — the navigation succeeded and the page is usually usable, so erroring would leave the agent with nothing to act on. A probe that throws mid-navigation (destroyed execution context) is retried until the deadline rather than propagating. Five unit tests plus an integration case.

7. SessionManager was a one-shot

closeAll() latched shuttingDown on permanently, so every later register() threw "SessionManager is shutting down" — a library host that tears the tool family down and brings it back up in the same process got a silently dead manager.

The flag is now held only for the duration of the sweep (registrations during teardown are still rejected — tested), and register() restarts the reaper closeAll() stopped. Without that restart a revived manager's sessions would idle until TestingBot's own reaper billed them out.

Two existing tests asserted the old latching behaviour and have been rewritten to encode the new contract.

8. Secrets in logs

The CallTool handler logs { tool, args } wholesale. tb_type's text — whatever gets typed into a login form — was not redacted. Added args.text, args.capabilities (appium caps carry app credentials), args.remoteServerUrl (embeds key:secret), plus generic *.secret / *.token / *.accessKey.

Verified live: {"args":{"sessionId":"x","value":"#pw","text":"[redacted]"}}. tests/lib/logger.test.ts also asserts the non-secret fields stay readable — over-redacting makes the logs useless.

Only reachable under TESTINGBOT_DEBUG=true or dev mode, since the default level is error.

9. Locator and key edge cases

• A leading digit in an id can't be backslash-escaped; CSS requires the hex form (#2fa-code → #\32 fa-code).
• by: "class" with "btn primary" means the compound selector .btn.primary. It was producing one escaped class literally named btn primary.
• "+" is both the chord separator and a real key, so it was unpressable. "+" and "Control++" now work.

Verification

npm run build passes: lint, format, 156 unit tests (up from 120), tsc. Integration suites still skip cleanly without the gate flags.