refactor: eliminate duplicate log source resolution logic#2619
Conversation
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 86.81% | 86.88% | 📈 +0.07% |
| Statements | 86.75% | 86.82% | 📈 +0.07% |
| Functions | 81.39% | 80.71% | 📉 -0.68% |
| Branches | 79.55% | 79.63% | 📈 +0.08% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/container-lifecycle.ts |
87.1% → 88.2% (+1.14%) | 87.5% → 88.6% (+1.11%) |
src/commands/logs-command-helpers.ts |
84.7% → 88.3% (+3.59%) | 85.0% → 88.5% (+3.52%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
Smoke Test Results✅ GitHub MCP: Last 2 merged PRs retrieved Status: PASS
|
🔥 Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Overall: PARTIAL — BYOK inference path confirmed working; pre-computed step data was not substituted into prompt. PR author:
|
🔬 Smoke Test Results
Overall: PASS Author:
|
|
Smoke Test Codex: FAIL 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.
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the awf logs command to reuse the existing log source discovery/validation/selection helper instead of maintaining a duplicated inline implementation, reducing drift risk between log-related commands.
Changes:
- Made
discoverAndSelectSource()accept an optionalloggingOptionsparameter to preservelogsCommand’s unconditional info logging behavior when auto-selecting a source. - Replaced the inline source discovery/selection logic in
logs.tswith a call todiscoverAndSelectSource(options.source). - Updated
logs-audit.test.tstyping assertions to account for the now-optional helper parameter.
Show a summary per file
| File | Description |
|---|---|
src/commands/logs.ts |
Removes duplicated source-selection logic and delegates to discoverAndSelectSource(). |
src/commands/logs-command-helpers.ts |
Makes loggingOptions optional and adjusts conditional info logging behavior accordingly. |
src/commands/logs-audit.test.ts |
Updates test assertions for the optional loggingOptions parameter typing. |
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: 2
| * @param sourceOption - User-specified source path or "running", or undefined for auto-discovery | ||
| * @param loggingOptions - Options controlling when to emit log messages | ||
| * @param loggingOptions - Options controlling when to emit log messages; when omitted, info messages are always emitted | ||
| * @returns Selected log source | ||
| */ | ||
| export async function discoverAndSelectSource( | ||
| sourceOption: string | undefined, | ||
| loggingOptions: LoggingOptions | ||
| loggingOptions?: LoggingOptions | ||
| ): Promise<LogSource> { |
| listLogSources, | ||
| LogFormatter, | ||
| streamLogs, | ||
| } from '../logs'; | ||
| import { discoverAndSelectSource } from './logs-command-helpers'; | ||
|
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Chroot Version Comparison
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL —
|
|
@copilot address the review feedback |
logs.tscontained a ~38-line inline copy of the source discovery/validation/selection block that already existed asdiscoverAndSelectSource()inlogs-command-helpers.ts. Any bug fix or behavioral change had to be applied twice.Changes
logs-command-helpers.ts: MadeloggingOptionsoptional indiscoverAndSelectSource(). When omitted, info messages are always emitted — preserving the unconditional logging behavior expected bylogsCommand.logs.ts: Replaced the inline block with a single call todiscoverAndSelectSource(options.source); removed now-unused imports (discoverLogSources,selectMostRecent,validateSource).logs-audit.test.ts: AddedtoBeDefined()+ non-null assertion onloggingOptionsto satisfy TypeScript's stricter typing of the now-optional parameter.