Skip to content

fix(tests): add missing headers to diffs index.test.ts localReq mock#39047

Closed
ademczuk wants to merge 1 commit intoopenclaw:mainfrom
ademczuk:fix/diffs-test-localreq-headers
Closed

fix(tests): add missing headers to diffs index.test.ts localReq mock#39047
ademczuk wants to merge 1 commit intoopenclaw:mainfrom
ademczuk:fix/diffs-test-localreq-headers

Conversation

@ademczuk
Copy link
Copy Markdown
Contributor

@ademczuk ademczuk commented Mar 7, 2026

Summary

  • Problem: extensions/diffs/index.test.ts throws TypeError: Cannot read properties of undefined (reading 'x-forwarded-for') in the applies plugin-config defaults test case.
  • Why it matters: Every open PR currently fails the checks (node, extensions) CI job due to this, even when the PR doesn't touch extensions code. Blocks merge readiness assessment for unrelated PRs.
  • What changed: Added headers: {} to the localReq() mock helper so hasProxyForwardingHints() can safely access req.headers.
  • What did NOT change: No production code touched. Only the test mock helper in index.test.ts. The identical localReq() helper in http.test.ts already has this property (added in 44881b0).

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

User-visible / Behavior Changes

None. Test-only change.

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Repro + Verification

Environment

  • CI runner (Ubuntu, node)
  • Reproducible on any branch based on main after 44881b0

Steps

  1. Open any PR targeting main
  2. Wait for checks (node, extensions, pnpm test:extensions) CI job
  3. Observe failure in extensions/diffs/index.test.ts

Expected

All 258 extension test files pass (2260 tests).

Actual

1 test file fails, 257 pass. The specific failure:

TypeError: Cannot read properties of undefined (reading 'x-forwarded-for')
 at hasProxyForwardingHints  extensions/diffs/src/http.ts:180:9
 at resolveViewerAccess      extensions/diffs/src/http.ts:193:58
 at                          extensions/diffs/index.test.ts:124:27

Evidence

Before (from PR #38631 CI run 22803165182):

FAIL extensions/diffs/index.test.ts > diffs plugin registration > applies plugin-config defaults
TypeError: Cannot read properties of undefined (reading 'x-forwarded-for')
Test Files  1 failed | 257 passed (258)
Tests       1 failed | 2258 passed | 1 skipped (2260)

After (PR #39047 CI run 22803786316):

checks (node, extensions, pnpm test:extensions)  pass  1m57s

Human Verification (required)

  • Verified the localReq() helper in http.test.ts:235-244 already includes headers: input.headers ?? {} as the established pattern from the same commit (44881b0)
  • Confirmed index.test.ts's localReq() is the only other mock helper that was missed
  • Checked the full index.test.ts file: localReq() is called once (line 125), always with { method, url }, never needs custom headers
  • CI confirms fix: extensions job now passes on this PR

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No

Failure Recovery (if this breaks)

  • Revert this single commit
  • Zero risk: only adds an empty object to a test mock

Risks and Mitigations

None. Single-line addition to a test helper with no production code impact.

The `localReq()` helper in `index.test.ts` was missing a `headers`
property, causing `hasProxyForwardingHints()` to throw when accessing
`req.headers["x-forwarded-for"]` on an undefined object.

This was introduced in 44881b0 which hardened proxied local viewer
detection and updated `http.test.ts` but missed `index.test.ts`.

Mirrors the fix pattern already applied in `http.test.ts:242`.
@ademczuk
Copy link
Copy Markdown
Contributor Author

ademczuk commented Mar 7, 2026

@steipete - this fixes the test gap from your 44881b0 commit. The index.test.ts localReq mock was missing headers: {}, which your http.test.ts version already has.

1 line, test-only, mirrors your existing pattern. Should unblock all open PRs failing extensions CI.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 7, 2026

Greptile Summary

This PR fixes a failing test in extensions/diffs/index.test.ts by adding the missing headers: {} property to the localReq() mock helper. The omission caused a TypeError at runtime when hasProxyForwardingHints() (introduced in commit 44881b02) accessed req.headers on a mock IncomingMessage that had no headers key.

  • Fix: extensions/diffs/index.test.ts:146 — adds headers: {} to localReq(), exactly mirroring the pattern in extensions/diffs/src/http.test.ts:242.
  • Scope: Test-only, zero runtime impact.
  • Root cause: http.test.ts was updated when hasProxyForwardingHints() was added, but the separate localReq() helper in index.test.ts was overlooked.

The change is minimal, correct, and consistent with the existing codebase pattern.

Confidence Score: 5/5

  • This PR is safe to merge — it is a one-line test-only fix with no runtime impact.
  • The change is a single-line addition to a test helper that directly mirrors an already-established pattern in the same codebase. It fixes a clear, reproducible TypeError in the test suite without touching any production code. No regressions are possible.
  • No files require special attention

Last reviewed commit: 154c18c

Shennng added a commit to Shennng/openclaw that referenced this pull request Mar 7, 2026
Add headers property to localReq() mock helper in extensions/diffs/index.test.ts to match the pattern already used in http.test.ts (commit 44881b0). This fixes the test failure:

TypeError: Cannot read properties of undefined (reading 'x-forwarded-for')
  at hasProxyForwardingHints extensions/diffs/src/http.ts:180:9
  at resolveViewerAccess extensions/diffs/src/http.ts:193:58
  at extensions/diffs/index.test.ts:124:27

The hasProxyForwardingHints() function tries to access req.headers, but the mock in index.test.ts was missing this property while http.test.ts already had it.

Fixes openclaw#39047
@ademczuk
Copy link
Copy Markdown
Contributor Author

ademczuk commented Mar 7, 2026

Consolidating into #39041 which has clean merge state and full green CI. Upgraded that PR's description to the full template.

@ademczuk ademczuk closed this Mar 7, 2026
@steipete
Copy link
Copy Markdown
Contributor

steipete commented Mar 7, 2026

Duplicate triage result from top-100 open PR review.

Keeping #39063 open as canonical for this fix.

Why #39063 won:

  • same core patch (same target file and behavior fix)
  • cleaner merge signal right now (clean state)
  • full CI pass signal on the canonical branch

Closing this PR to keep one active thread for the same fix and reduce reviewer/CI duplication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants