refactor: split types.ts into domain-scoped modules#2497
Conversation
Move types from the monolithic src/types.ts into focused modules under src/types/: config, docker, policy, logging, and pid. A barrel index.ts re-exports everything for full backwards compatibility. Closes #2475 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Refactors the codebase’s TypeScript type definitions by replacing the former monolithic src/types.ts with domain-scoped modules under src/types/, while preserving existing import paths via a barrel src/types/index.ts.
Changes:
- Added domain modules for policy, logging, PID tracking, and Docker-related types under
src/types/. - Removed non-config type definitions from
src/types/config.ts(leaving config-focused types/constants there). - Introduced
src/types/index.tsto re-export all public types/constants for backward-compatible./types/../typesimports.
Show a summary per file
| File | Description |
|---|---|
| src/types/config.ts | Removes non-config types so this module focuses on wrapper/config-related types and constants. |
| src/types/docker.ts | Introduces Docker Compose and Squid-related configuration types (plus docs) in a dedicated module. |
| src/types/index.ts | Adds a barrel re-export to keep existing import paths stable. |
| src/types/logging.ts | Moves log-related types into a dedicated module. |
| src/types/pid.ts | Moves PID tracking result type into a dedicated module. |
| src/types/policy.ts | Moves policy rule/manifest types into a dedicated module. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 1
| volumes?: { | ||
| [key: string]: Record<string, unknown>; | ||
| }; |
|
@copilot address the review feedback |
…port Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/0f210f33-b44a-4f1a-b2d5-d1253403c4d7 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Added a |
Chroot Version Comparison Results
Result: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environments.
|
|
refactor: extract runWithSignalHandling helper 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.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Smoke Test Results
Overall: FAIL —
|
🤖 Smoke Test Results
PR: refactor: split types.ts into domain-scoped modules Overall: PASS (MCP and file I/O verified; HTTP step skipped due to missing pre-step data)
|
|
Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Overall: PASS (core BYOK path functional; pre-step data unavailable due to unsubstituted template vars) PR by @lpcox · Reviewer:
|
|
Smoke Test Results — All tests passed ✅
Status: PASS
|
Summary
Split the 1,810-line monolithic
src/types.tsinto domain-scoped type modules undersrc/types/.src/types/config.tssrc/types/docker.tssrc/types/policy.tssrc/types/logging.tssrc/types/pid.tssrc/types/index.tsBackwards compatibility
All existing
import { ... } from './types'statements work unchanged via the barrel re-export.Testing
Closes #2475