Skip to content

Narrow log-discovery API: make isContainerRunning test-helper only#3530

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

Narrow log-discovery API: make isContainerRunning test-helper only#3530
lpcox merged 2 commits into
mainfrom
copilot/remove-unused-export-iscontainerrunning

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 21, 2026

src/logs/log-discovery.ts exported isContainerRunning even though it is only used internally (plus white-box tests), which unnecessarily expanded the module API surface. This change removes it from the public export surface while preserving direct test access via the existing testHelpers pattern.

  • API surface cleanup

    • Removed export from isContainerRunning in src/logs/log-discovery.ts.
    • Kept behavior unchanged for internal call sites (discoverLogSources, validateSource).
  • Test-only access pattern

    • Added testHelpers export in log-discovery.ts:
      • export const testHelpers = { isContainerRunning };
    • Aligns with established repository convention for exposing internals to tests without promoting them to first-class API.
  • Test updates

    • Updated src/logs/log-discovery.test.ts to import testHelpers and call testHelpers.isContainerRunning(...) instead of importing isContainerRunning directly.
// log-discovery.ts
async function isContainerRunning(containerName: string): Promise<boolean> {
  // ...
}

/** @internal Exposed only for unit tests — not part of the public API. */
// ts-prune-ignore-next
export const testHelpers = { isContainerRunning };

Copilot AI changed the title [WIP] Remove unused export isContainerRunning from log-discovery Narrow log-discovery API: make isContainerRunning test-helper only May 21, 2026
Copilot finished work on behalf of lpcox May 21, 2026 16:09
Copilot AI requested a review from lpcox May 21, 2026 16:09
@lpcox lpcox marked this pull request as ready for review May 21, 2026 16:27
Copilot AI review requested due to automatic review settings May 21, 2026 16:27
@github-actions
Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 95.98% 96.06% 📈 +0.08%
Statements 95.81% 95.88% 📈 +0.07%
Functions 97.36% 97.36% ➡️ +0.00%
Branches 89.48% 89.52% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 83.0% → 85.6% (+2.54%) 83.0% → 85.6% (+2.54%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

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 pull request narrows the src/logs/log-discovery.ts module’s public API by making isContainerRunning module-private while still allowing white-box unit tests to call it via the repository’s established testHelpers export pattern.

Changes:

  • Removed the direct export from isContainerRunning, making it internal-only for production code paths.
  • Added export const testHelpers = { isContainerRunning } to preserve test access without promoting the function as first-class API.
  • Updated unit tests to reference testHelpers.isContainerRunning(...) instead of importing isContainerRunning directly.
Show a summary per file
File Description
src/logs/log-discovery.ts Makes isContainerRunning private and exposes it only via testHelpers for unit tests.
src/logs/log-discovery.test.ts Updates tests to use testHelpers.isContainerRunning after API surface narrowing.

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

Smoke Test Results

API verification: 2 recent PRs found
Playwright check: PASS — GitHub title correct
File verify: Test marker exists

PASS — Claude validation complete

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

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

Overall: PASS

PR: Narrow log-discovery API: make isContainerRunning test-helper only — author @Copilot, assignees @lpcox @Copilot

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

🔥 Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP (list PRs) ✅ PR #3522 fetched successfully
GitHub.com Connectivity ⚠️ Pre-step data not injected (${{ steps.smoke-data.outputs.SMOKE_HTTP_CODE }} unresolved)
File Write/Read ⚠️ Pre-step data not injected (SMOKE_FILE_PATH unresolved)
BYOK Inference (this response) ✅ Agent running via api-proxy → api.githubcopilot.com

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

Overall: PARTIAL — BYOK inference path works; pre-step smoke data was not interpolated into the prompt.

cc @lpcox (reviewer), @Copilot (assignee)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

🏗️ 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 #3530 · ● 5.7M ·

@github-actions
Copy link
Copy Markdown
Contributor

Smoke test result summary: FAIL

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

Smoke Codex: FAIL
PRs: Optimize Smoke Claude workflow token footprint by removing Playwright and constraining turns; Reduce test-coverage-improver token burn by narrowing file access and preloading target sources
✅ GitHub PR review
❌ safeinputs-gh query (tool unavailable)
✅ Playwright title check
❌ Tavily search (no tools exposed)
✅ file/bash check; ✅ discussion comment via fallback; ✅ npm build

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

Chroot Version Comparison Results

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

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

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test: GitHub Actions Services Connectivity

Check Result
Redis PING (host.docker.internal:6379) ❌ No response (timeout)
PostgreSQL pg_isready (host.docker.internal:5432) ❌ No response
PostgreSQL SELECT 1 ❌ Connection timeout

Overall: FAIL — Service containers are not reachable via host.docker.internal (resolves to 172.17.0.1 but no services responding on ports 6379/5432).

🔌 Service connectivity validated by Smoke Services

@lpcox lpcox merged commit 66c5d9b into main May 21, 2026
66 of 72 checks passed
@lpcox lpcox deleted the copilot/remove-unused-export-iscontainerrunning branch May 21, 2026 17:55
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] isContainerRunning exported but never used outside its own module

3 participants