Skip to content

fix: propagate apiProxy.diagnostics config fields to all layers#4743

Merged
lpcox merged 3 commits into
mainfrom
fix/propagate-diagnostics-config-fields-ed13488b36ed1335
Jun 11, 2026
Merged

fix: propagate apiProxy.diagnostics config fields to all layers#4743
lpcox merged 3 commits into
mainfrom
fix/propagate-diagnostics-config-fields-ed13488b36ed1335

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Config Consistency Fixes

Automated fixes for configuration fields not fully propagated across all required layers.

From PR #4678 — "Add opt-in diagnostics artifact for blocked LLM request bodies"

PR #4678 added apiProxy.diagnostics.captureBlockedRequests and apiProxy.diagnostics.maxCapturedBytes to both JSON schemas and the spec, but did not wire them through the TypeScript type system or the env var mapping.

Field Layer Fix Applied
apiProxy.diagnostics.captureBlockedRequests TypeScript type Added captureBlockedRequests?: boolean | 'summary' | 'redacted' | 'full' to src/types/api-proxy-options.ts
apiProxy.diagnostics.captureBlockedRequests Config file mapping Added diagnostics? nested interface + captureBlockedRequests mapping in src/config-file.ts
apiProxy.diagnostics.captureBlockedRequests Env var wiring Added AWF_CAPTURE_BLOCKED_LLM_REQUESTS wiring in src/services/api-proxy-service-config.ts
apiProxy.diagnostics.maxCapturedBytes TypeScript type Added maxCapturedBytes?: number to src/types/api-proxy-options.ts
apiProxy.diagnostics.maxCapturedBytes Config file mapping Added maxCapturedBytes mapping in src/config-file.ts
apiProxy.diagnostics.maxCapturedBytes Env var wiring Added AWF_MAX_BLOCKED_CAPTURE_BYTES wiring in src/services/api-proxy-service-config.ts

Layers already present (no fix needed)

Layer Status
JSON Schema (src/awf-config-schema.json) ✅ Added by PR #4678
JSON Schema (docs/awf-config.schema.json) ✅ Added by PR #4678 (identical to src)
Spec CLI mapping (docs/awf-config-spec.md) ✅ Added by PR #4678
api-proxy runtime (containers/api-proxy/blocked-request-diagnostics.js) ✅ Added by PR #4678

Verification

  • TypeScript compiles (tsc --noEmit) — clean
  • Config-file and api-proxy-service-config tests pass (167 tests, all green)

Generated by Config Consistency Auditor · 533.9 AIC · ⊞ 26.8K ·

PR #4678 added apiProxy.diagnostics.captureBlockedRequests and
apiProxy.diagnostics.maxCapturedBytes to the JSON schemas and spec,
but the fields were not wired through the TypeScript type system or
the env var mapping in api-proxy-service-config.ts.

Gaps fixed:
- src/types/api-proxy-options.ts: add captureBlockedRequests and
  maxCapturedBytes typed fields
- src/config-file.ts: add diagnostics nested interface and map
  apiProxy.diagnostics.* to the flat ApiProxyOptions fields
- src/services/api-proxy-service-config.ts: wire captureBlockedRequests
  → AWF_CAPTURE_BLOCKED_LLM_REQUESTS and maxCapturedBytes →
  AWF_MAX_BLOCKED_CAPTURE_BYTES env vars

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review June 11, 2026 16:54
Copilot AI review requested due to automatic review settings June 11, 2026 16:54
@github-actions

Copy link
Copy Markdown
Contributor Author

⚠️ Coverage Regression Detected

This PR decreases test coverage. Please add tests to maintain coverage levels.

Overall Coverage

Metric Base PR Delta
Lines 96.42% 96.46% 📈 +0.04%
Statements 96.34% 96.38% 📈 +0.04%
Functions 98.77% 98.77% ➡️ +0.00%
Branches 90.74% 90.69% 📉 -0.05%
📁 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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 aims to make the apiProxy.diagnostics.captureBlockedRequests and apiProxy.diagnostics.maxCapturedBytes config keys usable end-to-end by adding the missing TypeScript typings, config-file mapping, and api-proxy container env var wiring—closing gaps left after PR #4678 introduced the schema/spec/runtime support.

Changes:

  • Added captureBlockedRequests / maxCapturedBytes fields to the ApiProxyOptions TypeScript interface.
  • Extended src/config-file.ts to support an apiProxy.diagnostics section and map it into CLI option keys.
  • Updated api-proxy sidecar service env generation to emit AWF_CAPTURE_BLOCKED_LLM_REQUESTS and AWF_MAX_BLOCKED_CAPTURE_BYTES.
Show a summary per file
File Description
src/types/api-proxy-options.ts Adds TS option fields and inline docs for blocked-request diagnostics configuration.
src/config-file.ts Adds apiProxy.diagnostics config shape and maps it to flat option keys used by the CLI/config merging layer.
src/services/api-proxy-service-config.ts Forwards the new settings into api-proxy container environment variables.

Copilot's findings

Tip

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

  • Files reviewed: 3/3 changed files
  • Comments generated: 3

Comment on lines +196 to +200
...(config.captureBlockedRequests !== undefined &&
config.captureBlockedRequests !== false && {
AWF_CAPTURE_BLOCKED_LLM_REQUESTS: String(config.captureBlockedRequests),
}),
...(config.maxCapturedBytes !== undefined && {
Comment thread src/config-file.ts
Comment on lines 250 to +254
modelAliases: config.apiProxy?.models,
debugTokens: config.apiProxy?.logging?.debugTokens,
tokenLogDir: config.apiProxy?.logging?.tokenLogDir,
captureBlockedRequests: config.apiProxy?.diagnostics?.captureBlockedRequests,
maxCapturedBytes: config.apiProxy?.diagnostics?.maxCapturedBytes,
Comment on lines +199 to +202
}),
...(config.maxCapturedBytes !== undefined && {
AWF_MAX_BLOCKED_CAPTURE_BYTES: String(config.maxCapturedBytes),
}),
@lpcox

lpcox commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copilot AI commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@copilot address review feedback

Addressed all three review comments in commit fix: wire captureBlockedRequests in buildConfig; add tests:

  1. build-config.ts gap — Added captureBlockedRequests and maxCapturedBytes to the buildConfig() return value, reading from CLI options with process.env.AWF_CAPTURE_BLOCKED_LLM_REQUESTS / process.env.AWF_MAX_BLOCKED_CAPTURE_BYTES as fallbacks. Values now flow end-to-end from config file or env → WrapperConfig → api-proxy container env vars.

  2. config-file-mapping.test.ts coverage — Added four new test cases covering captureBlockedRequests (string mode, boolean true) and maxCapturedBytes mappings, plus a case asserting both are undefined when diagnostics is absent.

  3. api-proxy-service-misc-forwarding.test.ts coverage — Added five new test cases asserting AWF_CAPTURE_BLOCKED_LLM_REQUESTS and AWF_MAX_BLOCKED_CAPTURE_BYTES are correctly set (or absent when captureBlockedRequests: false) in the generated docker-compose env.

Copilot finished work on behalf of lpcox June 11, 2026 17:05
Copilot AI requested a review from lpcox June 11, 2026 17:05
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@lpcox

lpcox commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Copilot AI commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@copilot fix this failing ci check https://github.com/github/gh-aw-firewall/actions/runs/27363917538/job/80858488872?pr=4743

Fixed in commit test: add build-config coverage for captureBlockedRequests env vars. The branch coverage regression was caused by the new conditional expressions in build-config.ts not having test coverage. Added 6 test cases to src/commands/build-config.test.ts (plus added the two new env var keys to the ENV_KEYS snapshot array) covering:

  • Reading AWF_CAPTURE_BLOCKED_LLM_REQUESTS from process.env
  • CLI option taking precedence over the env var
  • Both being absent → undefined
  • Same three variants for AWF_MAX_BLOCKED_CAPTURE_BYTES / maxCapturedBytes

Copilot finished work on behalf of lpcox June 11, 2026 17:34
@github-actions

Copy link
Copy Markdown
Contributor Author

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.42% 96.47% 📈 +0.05%
Statements 96.34% 96.38% 📈 +0.04%
Functions 98.77% 98.77% ➡️ +0.00%
Branches 90.74% 90.88% 📈 +0.14%
📁 Per-file Coverage Changes (3 files)
File Lines (Before → After) Statements (Before → After)
src/commands/validators/config-assembly.ts 98.2% → 98.3% (+0.04%) 97.4% → 97.5% (+0.06%)
src/squid/policy-manifest.ts 87.0% → 87.2% (+0.28%) 87.2% → 87.5% (+0.27%)
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

@github-actions

Copy link
Copy Markdown
Contributor Author

Smoke Test: Copilot PAT Auth — PASS ✅

Test Result
GitHub MCP connectivity
GitHub.com HTTP ✅ 200
File write/read

PR: fix: propagate apiProxy.diagnostics config fields to all layers
Author: @github-actions[bot] · Reviewer: @lpcox
Auth mode: PAT (COPILOT_GITHUB_TOKEN)

🔑 PAT report filed by Smoke Copilot PAT

@github-actions

Copy link
Copy Markdown
Contributor Author

@github-actions[bot]

  • fix: propagate apiProxy.diagnostics config fields to all layers

MCP connectivity: ✅
GitHub.com HTTP: ✅
File I/O: ✅
Direct BYOK inference: ✅

Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)

Overall: PASS

🔑 BYOK (AOAI api-key) report filed by Smoke Copilot BYOK AOAI (api-key)

@github-actions

Copy link
Copy Markdown
Contributor Author

GitHub API: ✅ PASS
GitHub check: ✅ PASS
File verify: ✅ PASS

Total: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions

Copy link
Copy Markdown
Contributor Author

🔬 Smoke Test Results

PR: fix: propagate apiProxy.diagnostics config fields to all layers
Author: @github-actions[bot] | Reviewer: @lpcox

Test Result
GitHub MCP connectivity
GitHub.com HTTP connectivity ❌ (pre-step template vars not substituted)
File write/read ❌ (pre-step template vars not substituted)

Overall: FAIL — pre-step outputs were not resolved; tests 2 & 3 could not be verified.

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor Author

Smoke Test: Copilot BYOK (Direct) Mode

Status: ✅ PASS

Test Result
MCP Connectivity
GitHub.com (HTTP 200)
File Write/Read
BYOK Inference

Mode: Direct BYOK (COPILOT_PROVIDER_API_KEY) via api-proxy sidecar → api.githubcopilot.com

Related: PR #4764, #4765 · Author: @github-actions[bot] · Assigned: @lpcox, @Copilot

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions

Copy link
Copy Markdown
Contributor Author

Merged PRs:

  • docs: sync schemas and specs with source changes
  • [Test Coverage] host-iptables branch coverage

Checks:

  • GitHub reads: ✅
  • Playwright title check: ✅
  • File write/read: ✅
  • Discussion comment: ✅
  • Build npm ci && npm run build: ❌

Overall: FAIL

🔮 The oracle has spoken through Smoke Codex

@github-actions

Copy link
Copy Markdown
Contributor Author

Smoke Test: GitHub Actions Services Connectivity

Running inside AWF sandbox (172.30.0.20). host.docker.internal resolves to 172.17.0.1.

Check Result
Redis PING ❌ timeout
PostgreSQL pg_isready ❌ no response
PostgreSQL SELECT 1 ❌ timeout

Overall: FAIL

Root cause: AWF iptables rules block outbound connections to database ports (Redis 6379, PostgreSQL 5432) as "dangerous ports".

🔌 Service connectivity validated by Smoke Services

@github-actions

Copy link
Copy Markdown
Contributor Author

Chroot Version Comparison Results

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

Overall: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.

Tested by Smoke Chroot

@github-actions

Copy link
Copy Markdown
Contributor Author

fix: propagate apiProxy.diagnostics config fields to all layers

  1. GitHub MCP: ✅
  2. GitHub.com HTTP: ✅ (code 200)
  3. File read/write: ✅
  4. BYOK inference: ✅
    Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra
    Overall: PASS

Warning

Firewall blocked 1 domain

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

  • api.openai.com

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

network:
  allowed:
    - defaults
    - "api.openai.com"

See Network Configuration for more information.

🪪 BYOK (AOAI Entra) report filed by Smoke Copilot BYOK AOAI (Entra)

@lpcox lpcox merged commit 0afb14c into main Jun 11, 2026
79 of 80 checks passed
@lpcox lpcox deleted the fix/propagate-diagnostics-config-fields-ed13488b36ed1335 branch June 11, 2026 17:59
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.

3 participants