Skip to content

Narrow log parser API by making extractDomain internal#3823

Merged
lpcox merged 2 commits into
mainfrom
copilot/remove-unused-export-extractdomain
May 26, 2026
Merged

Narrow log parser API by making extractDomain internal#3823
lpcox merged 2 commits into
mainfrom
copilot/remove-unused-export-extractdomain

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 26, 2026

extractDomain in src/logs/log-parser.ts was still exported even though it is only used internally and by tests. This PR removes that dead public export and updates tests to validate domain extraction through public parser behavior instead of a private helper.

  • API surface cleanup

    • Removed extractDomain from the module’s public exports.
    • Kept all internal call sites (parseLogLine, parseAuditJsonlLine) unchanged.
  • Test refactor to public contract

    • Dropped direct extractDomain imports from log-parser.test.ts.
    • Replaced helper-level unit assertions with parseLogLine cases that cover:
      • CONNECT domain extraction (with/without port)
      • host-header extraction for non-CONNECT requests
      • URL fallback paths (with/without protocol)
      • invalid URL fallback behavior
// before
export function extractDomain(url: string, host: string, method: string): string

// after
function extractDomain(url: string, host: string, method: string): string

Copilot AI changed the title [WIP] Remove unused export extractDomain from log-parser.ts Narrow log parser API by making extractDomain internal May 26, 2026
Copilot AI requested a review from lpcox May 26, 2026 00:09
Copilot finished work on behalf of lpcox May 26, 2026 00:09
@lpcox lpcox marked this pull request as ready for review May 26, 2026 00:45
Copilot AI review requested due to automatic review settings May 26, 2026 00:45
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 narrows the public API of the Squid firewall log parser by making extractDomain an internal helper, and updates tests to validate domain extraction through the public parsing functions instead of importing the helper directly.

Changes:

  • Made extractDomain non-exported (module-internal) in src/logs/log-parser.ts.
  • Refactored log-parser tests to assert domain extraction via parseLogLine outputs rather than extractDomain calls.
Show a summary per file
File Description
src/logs/log-parser.ts Removes the public export for extractDomain while keeping internal behavior unchanged.
src/logs/log-parser.test.ts Updates tests to cover domain extraction through parseLogLine (public contract) instead of importing extractDomain.

Copilot's findings

Tip

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

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

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 96.54% 96.58% 📈 +0.04%
Statements 96.38% 96.42% 📈 +0.04%
Functions 97.99% 97.99% ➡️ +0.00%
Branches 90.78% 90.82% 📈 +0.04%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/logs/log-parser.ts 88.0% → 87.8% (-0.15%) 87.0% → 86.9% (-0.15%)
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

ghost commented May 26, 2026

Smoke Test: Claude Engine

Result: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

🔬 Smoke Test Results

Test Status
GitHub MCP connectivity ✅ PR #3823 fetched successfully
GitHub.com HTTP connectivity ⚠️ Unreplaced template — skipped
File write/read ⚠️ Unreplaced template — skipped

PR: "Narrow log parser API by making extractDomain internal" — author: @Copilot, assignees: @lpcox, @Copilot

Overall: PARTIAL — MCP test passed; pre-step data unavailable (template variables not substituted).

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP connectivity ✅ PR list fetched successfully
GitHub.com HTTP ⚠️ Pre-step data not expanded (template vars unavailable)
File write/read ⚠️ Pre-step data not expanded (template vars unavailable)
BYOK inference (this response) ✅ Agent responding via api-proxy → api.githubcopilot.com

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com.

PR: "Narrow log parser API by making extractDomain internal" — author @Copilot, assignees @lpcox @Copilot

Overall: PARTIAL (2 tests verified ✅, 2 tests unverifiable due to unexpanded template variables)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

  • fix(api-proxy): prevent stream_options injection into OpenAI Responses API requests ✅
  • Filter unresolvable model aliases from /reflect and models.json ✅
  • GitHub title check ✅
  • Build ✅
  • Overall: 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

ghost commented May 26, 2026

🧪 Chroot Runtime Version Comparison

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

Result: ❌ Not all runtimes match — Python and Node.js versions differ between host and chroot environment.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 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 all passed ✅ PASS
Node.js execa all passed ✅ PASS
Node.js p-limit all 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 #3823 · sonnet46 3.3M ·

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

Gemini Engine Smoke Test Results

Overall Status: PASS

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

@github-actions
Copy link
Copy Markdown
Contributor

ghost commented May 26, 2026

Smoke Test Results

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

Overall: FAILhost.docker.internal is not reachable from this runner environment. Service containers may not be configured or network routing is unavailable.

🔌 Service connectivity validated by Smoke Services

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 26, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 26, 2026

@copilot address review feedback

I checked the PR review data and there are currently no actionable review comments/threads to address (the review summary shows “Comments generated: 0”). If you want a specific change, please point me to the comment or describe the requested update and I’ll apply it right away.

Copilot finished work on behalf of lpcox May 26, 2026 02:21
@lpcox lpcox merged commit c844644 into main May 26, 2026
64 of 67 checks passed
@lpcox lpcox deleted the copilot/remove-unused-export-extractdomain branch May 26, 2026 02:33
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.

[Export Audit] Dead export: extractDomain exported from log-parser.ts but never used in production code

3 participants