Skip to content

Deduplicate JSON body object parsing across api-proxy transforms and model rewrite paths#4337

Open
Copilot wants to merge 3 commits into
mainfrom
copilot/duplicate-json-guard-refactor
Open

Deduplicate JSON body object parsing across api-proxy transforms and model rewrite paths#4337
Copilot wants to merge 3 commits into
mainfrom
copilot/duplicate-json-guard-refactor

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 4, 2026

The API proxy repeated the same JSON body parse-and-validate guard across five request-processing functions, creating drift risk for future hardening or behavior changes. This change centralizes that guard and updates all identified call sites to use it.

  • Shared body parsing utility

    • Added containers/api-proxy/body-utils.js with parseBodyAsObject(body).
    • Standardizes behavior for:
      • invalid JSON
      • non-object JSON values
      • array payloads
  • Call site refactors (5 sites)

    • containers/api-proxy/body-transform.js
      • sanitizeNullToolCallTypes
      • injectSteeringMessage
      • injectStreamOptions
    • containers/api-proxy/providers/copilot.js
      • injectByokExtraBodyFields
    • containers/api-proxy/model-resolver.js
      • rewriteModelInBody
  • Focused coverage for shared behavior

    • Added containers/api-proxy/body-utils.test.js to assert object parse success and null-return behavior for invalid/non-object inputs.
const { parseBodyAsObject } = require('./body-utils');

const parsed = parseBodyAsObject(body);
if (!parsed) return null;

Copilot AI changed the title [WIP] Refactor duplicate JSON body parse-and-validate guard Deduplicate JSON body object parsing across api-proxy transforms and model rewrite paths Jun 4, 2026
Copilot finished work on behalf of lpcox June 4, 2026 23:19
Copilot AI requested a review from lpcox June 4, 2026 23:19
@lpcox lpcox marked this pull request as ready for review June 4, 2026 23:46
Copilot AI review requested due to automatic review settings June 4, 2026 23:46
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.59% 96.63% 📈 +0.04%
Statements 96.50% 96.54% 📈 +0.04%
Functions 98.71% 98.71% ➡️ +0.00%
Branches 90.86% 90.86% ➡️ +0.00%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 89.3% → 90.9% (+1.65%) 89.3% → 90.9% (+1.65%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces drift risk in the API proxy by centralizing the repeated “parse JSON body and ensure it’s a non-array object” guard into a shared utility, then refactoring the existing body-transform and model-rewrite call sites to use it.

Changes:

  • Added parseBodyAsObject(body) in containers/api-proxy/body-utils.js to standardize JSON object parsing and validation.
  • Refactored five call sites across body-transform.js, providers/copilot.js, and model-resolver.js to use the shared parser.
  • Added focused Jest coverage for the shared parsing behavior in containers/api-proxy/body-utils.test.js.
Show a summary per file
File Description
containers/api-proxy/providers/copilot.js Switches BYOK extra-body-field injection to use the shared JSON object parser.
containers/api-proxy/model-resolver.js Switches model rewrite path to use the shared JSON object parser.
containers/api-proxy/body-transform.js Switches transform utilities to use the shared JSON object parser.
containers/api-proxy/body-utils.js Introduces the centralized parseBodyAsObject helper.
containers/api-proxy/body-utils.test.js Adds unit tests to lock in shared parsing behavior.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 5/5 changed files
  • Comments generated: 0

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 5, 2026

@copilot fix this failing ci check https://github.com/github/gh-aw-firewall/actions/runs/26985429834/job/79637011619?pr=4337

The root cause was that body-utils.js was added to the repository but not listed in the Dockerfile's COPY instruction, so the api-proxy container couldn't find the module at runtime (Error: Cannot find module './body-utils'). Fixed in the latest commit by adding body-utils.js to the COPY list in containers/api-proxy/Dockerfile.

Copilot finished work on behalf of lpcox June 5, 2026 00:23
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

Smoke Test: Claude Engine

  • ✅ GitHub API: 2 PR entries found
  • ✅ GitHub check: playwright_check PASS
  • ✅ File verify: smoke-test-claude-26987699731.txt exists

Result: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

Smoke: Copilot BYOK (Direct) — PASS

Test Result
GitHub MCP connectivity
GitHub.com connectivity
File write/read (smoke-test-copilot-byok-26987699753.txt)
BYOK inference (api-proxy → api.githubcopilot.com)

Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) via api-proxy → api.githubcopilot.com.
PR by @Copilot, assignees: @lpcox, @Copilot.

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

PR titles:

  • Deduplicate API proxy config env lookup and auth type normalization
  • Refactor container startup-failure detection into shared helpers

✅ GitHub PR query
✅ Playwright title check
✅ File write verification
✅ Discussion lookup
✅ Build (npm ci && npm run build)

Overall status: PASS

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

🧪 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP (200)
File write/read

PR: Deduplicate JSON body object parsing across api-proxy transforms and model rewrite paths
Author: @Copilot · Assignees: @lpcox, @Copilot

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

🔬 Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Details
1. Module Loading ✅ Pass otel.js loads and exports startRequestSpan, setTokenAttributes, endSpan, endSpanError, shutdown, isEnabled
2. Test Suite ✅ Pass 33 tests across 8 describe blocks (init, header parsing, span creation, token attrs, span end/error, OTLP serialization, proxy exporter)
3. Env Var Forwarding ✅ Pass api-proxy-service-config.ts forwards OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID, OTEL_SERVICE_NAME to sidecar
4. Token Tracker Integration ✅ Pass onUsage callback present in token-tracker-http.js (line 268) — invoked after normalized usage extraction, wired as the OTEL hook point
5. OTEL Diagnostics ✅ Pass (graceful degradation) No OTLP endpoint configured → FileSpanExporter writes spans to /var/log/api-proxy/otel.jsonl; no network export, no errors

All scenarios passed. OTEL tracing integration is complete and working correctly.

📡 OTel tracing validated by Smoke OTel Tracing

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

🧪 Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3
Node.js v24.16.0 v22.22.3
Go go1.22.12 go1.22.12

Overall: ❌ Not all versions match.

Go versions are in sync. Python and Node.js differ between host and chroot environments.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color 1/1 passed ✅ PASS
Go env 1/1 passed ✅ PASS
Go uuid 1/1 passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx passed ✅ PASS
Node.js execa passed ✅ PASS
Node.js p-limit passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #4337 · sonnet46 1.8M ·

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

Smoke Test: GitHub Actions Services Connectivity

Check Result
Redis PING ❌ Timeout/no response
PostgreSQL pg_isready ❌ No response
PostgreSQL SELECT 1 ❌ Timeout/error

Overall: FAILhost.docker.internal services unreachable (connection timed out/no response on both ports 6379 and 5432).

🔌 Service connectivity validated by Smoke Services

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

Smoke Test: 1/4 PASS (Connectivity, File, Bash OK; MCP FAIL). Overall: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Duplicate Code] JSON body parse-and-validate guard copy-pasted into five api-proxy functions

3 participants