Narrow log parser API by making extractDomain internal#3823
Conversation
extractDomain internal
There was a problem hiding this comment.
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
extractDomainnon-exported (module-internal) insrc/logs/log-parser.ts. - Refactored
log-parsertests to assert domain extraction viaparseLogLineoutputs rather thanextractDomaincalls.
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
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (2 files)
Coverage comparison generated by |
Smoke Test: Claude Engine
Result: PASS
|
🔬 Smoke Test Results
PR: "Narrow log parser API by making Overall: PARTIAL — MCP test passed; pre-step data unavailable (template variables not substituted).
|
Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( PR: "Narrow log parser API by making Overall: PARTIAL (2 tests verified ✅, 2 tests unverifiable due to unexpanded template variables)
|
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
🧪 Chroot Runtime Version Comparison
Result: ❌ Not all runtimes match — Python and Node.js versions differ between host and chroot environment.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Gemini Engine Smoke Test Results
Overall Status: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
Smoke Test Results
Overall: FAIL —
|
|
@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. |
extractDomaininsrc/logs/log-parser.tswas 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
extractDomainfrom the module’s public exports.parseLogLine,parseAuditJsonlLine) unchanged.Test refactor to public contract
extractDomainimports fromlog-parser.test.ts.parseLogLinecases that cover: