Skip to content

Split docker-manager cleanup tests by concern#5357

Merged
lpcox merged 3 commits into
mainfrom
copilot/refactor-split-docker-manager-cleanup
Jun 21, 2026
Merged

Split docker-manager cleanup tests by concern#5357
lpcox merged 3 commits into
mainfrom
copilot/refactor-split-docker-manager-cleanup

Conversation

Copilot AI commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

src/docker-manager-cleanup.test.ts had grown into a mixed suite covering config generation, teardown, and diagnostics/audit preservation in one file. This change separates those independent concerns so each area is easier to locate, extend, and run in isolation.

  • Config-writing tests

    • Moved writeConfigs coverage into src/docker-manager-write-configs.test.ts
    • Kept the existing assertions for workdir creation, generated config files, permissions, proxy log paths, and chroot-home precreation
  • Cleanup tests

    • Reduced src/docker-manager-cleanup.test.ts to teardown-focused behavior
    • Retained lifecycle and artifact-preservation coverage for workdir removal, preserved logs, proxy log chmod, session state handling, and diagnostics preservation during cleanup
  • Diagnostics and audit tests

    • Moved collectDiagnosticLogs and preserveIptablesAudit coverage into src/docker-manager-diagnostics.test.ts
    • Kept the existing redaction, container log/state/mount capture, and audit-file preservation assertions intact
  • Resulting layout

    // before
    src/docker-manager-cleanup.test.ts
    
    // after
    src/docker-manager-write-configs.test.ts
    src/docker-manager-cleanup.test.ts
    src/docker-manager-diagnostics.test.ts

Copilot AI changed the title [WIP] Refactor split src/docker-manager-cleanup.test.ts by concern group Split docker-manager cleanup tests by concern Jun 21, 2026
Copilot AI requested a review from lpcox June 21, 2026 16:10
Copilot finished work on behalf of lpcox June 21, 2026 16:10
@lpcox
lpcox marked this pull request as ready for review June 21, 2026 17:12
Copilot AI review requested due to automatic review settings June 21, 2026 17:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the Docker manager-related unit tests by splitting a previously mixed suite into three focused test files, making config-writing, cleanup, and diagnostics/audit preservation behaviors easier to find and run independently.

Changes:

  • Added src/docker-manager-write-configs.test.ts to cover writeConfigs-related behavior.
  • Reduced src/docker-manager-cleanup.test.ts to teardown-focused tests (plus cleanup-time diagnostics preservation).
  • Added src/docker-manager-diagnostics.test.ts to cover collectDiagnosticLogs and preserveIptablesAudit.
Show a summary per file
File Description
src/docker-manager-write-configs.test.ts New test suite for writeConfigs behaviors (workdir creation, config outputs, permissions, proxy logs dir, chroot-home prep).
src/docker-manager-cleanup.test.ts Trimmed to focus on cleanup() teardown and artifact-preservation behavior.
src/docker-manager-diagnostics.test.ts New test suite for diagnostics log collection and iptables audit preservation behavior.

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: 4

expect((stats.mode & 0o777).toString(8)).toBe('700');
});

it('should write config files with restricted permissions (0o600)', async () => {
Comment thread src/docker-manager-write-configs.test.ts
Comment thread src/docker-manager-write-configs.test.ts
Comment on lines +260 to +264
const originalHome = process.env.HOME;
const originalSudoUser = process.env.SUDO_USER;
process.env.HOME = fakeHome;
delete process.env.SUDO_USER;

@lpcox

lpcox commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

Copilot AI commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Done in the latest commit. Four issues addressed in src/docker-manager-write-configs.test.ts:

  1. Misleading test name — renamed "restricted permissions (0o600)" → "correct permissions (squid.conf: 0o644, docker-compose.yml: 0o600)" to match the actual assertions.
  2. squid.conf test — removed the try/catch + if (existsSync) guard; the test now awaits writeConfigs directly and unconditionally asserts both file existence and content.
  3. docker-compose.yml test — same fix: always asserts the file is written.
  4. process.env restoration — both chroot-home tests now use try/finally so HOME and SUDO_USER are restored even when an assertion throws.

Copilot finished work on behalf of lpcox June 21, 2026 17:28
@github-actions

Copy link
Copy Markdown
Contributor

✅ Copilot review passed with no inline comments.

@copilot Add the ready-for-aw label to this PR to trigger agentic CI smoke tests.

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK AOAI (Entra) completed. Copilot AOAI BYOK (Entra) mode operational. 🔓

No file at expected path for write/read test

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded.

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Smoke Gemini completed. All facets verified. 💎

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Build Test Suite completed successfully!

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Smoke Copilot BYOK AOAI (api-key) completed. Copilot AOAI BYOK (api-key) mode operational. 🔓

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Smoke Claude passed

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

🔌 Smoke Services — All services reachable! ✅

@github-actions

Copy link
Copy Markdown
Contributor

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 97.85% 97.89% 📈 +0.04%
Statements 97.78% 97.82% 📈 +0.04%
Functions 99.50% 99.50% ➡️ +0.00%
Branches 93.55% 93.58% 📈 +0.03%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/workdir-setup.ts 92.7% → 94.5% (+1.82%) 92.7% → 94.5% (+1.82%)

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

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Claude Engine Validation

Check Status
API ✅ PASS
gh CLI ✅ PASS
File access ✅ PASS

Overall result: PASS

Generated by Smoke Claude for issue #5357 · 36.9 AIC · ⊞ 3.1K ·

@github-actions

Copy link
Copy Markdown
Contributor

Copilot BYOK Smoke Test: PASS

  • ✅ GitHub MCP connectivity verified
  • ✅ GitHub.com HTTP 200
  • ✅ File write/read test passed
  • ✅ BYOK inference working (direct mode via api-proxy → api.githubcopilot.com)

Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) with api-proxy sidecar injection.

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions

Copy link
Copy Markdown
Contributor

Smoke test

  • Split docker-manager cleanup tests by concern
  • perf(security-guard): prioritize security-relevant files in PR diff
  • fix(smoke-claude): raise turn budget to 8 and fix add_comment usage
  • GitHub page title: ✅
  • File write/read: ✅
  • 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

🔥 Smoke Test Results — Auth mode: PAT (COPILOT_GITHUB_TOKEN)

PR: Split docker-manager cleanup tests by concern
Author: @Copilot | Assignees: @lpcox @Copilot

Test Result
GitHub MCP connectivity
github.com HTTP (200)
File write/read

Overall: PASS

🔑 PAT report filed by Smoke Copilot PAT

@github-actions

Copy link
Copy Markdown
Contributor

🔭 Smoke Test: API Proxy OpenTelemetry Tracing

Scenario Result Detail
1 — Module loading otel.js loads and exports startRequestSpan, setTokenAttributes, setBudgetAttributes, endSpan, endSpanError, shutdown, isEnabled
2 — Test suite 59 passed, 0 failed across 2 suites (otel.test.js, otel-fanout.test.js)
3 — Env var forwarding api-proxy-service.ts forwards GH_AW_OTLP_ENDPOINTS, OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_HEADERS, GITHUB_AW_OTEL_TRACE_ID, GITHUB_AW_OTEL_PARENT_SPAN_ID, OTEL_SERVICE_NAME
4 — Token tracker integration onUsage callback present in token-tracker-http.js; proxy-request.js calls otel.startRequestSpan / setTokenAttributes / endSpan
5 — OTEL diagnostics No OTLP endpoint configured → spans written to /var/log/api-proxy/otel.jsonl fallback (graceful degradation works)

All scenarios pass. ✅

📡 OTel tracing validated by Smoke OTel Tracing

@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 ❌ No
Node.js v24.16.0 v22.22.3 ❌ No
Go go1.22.12 go1.22.12 ✅ Yes

Overall: ❌ Not all runtimes match — Go matches, but Python and Node.js versions differ between host and chroot.

Tested by Smoke Chroot

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test Results: MCP: ❌, Connectivity: ❌, File Writing: ✅, Bash Tool: ✅. Overall: 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 Test Results

Test Status
GitHub MCP connectivity ✅ PASS
HTTP connectivity (github.com) ❌ FAIL (pre-computed data unavailable)
File write/read ❌ FAIL (pre-computed data unavailable)

Overall: ❌ FAIL — pre-step template variables were not substituted (${{ steps.smoke-data.outputs.* }}), indicating the smoke-data step did not run or failed.

PR: Split docker-manager cleanup tests by concern | Author: @Copilot | Assignees: @lpcox @Copilot

📰 BREAKING: Report filed by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

@lpcox

Smoke Test Results:

  • MCP list PRs: ✅
  • GitHub.com HTTP: ✅
  • File write/read: ❌
  • Direct BYOK inference: ✅

Status: FAIL

Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra

🪪 BYOK (AOAI Entra) report filed by Smoke Copilot BYOK AOAI (Entra)

@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 #5357 · 149.8 AIC · ⊞ 7.7K ·

@github-actions

Copy link
Copy Markdown
Contributor

@Copilot @lpcox
Split docker-manager cleanup tests by concern
✅ MCP connectivity
✅ GitHub.com connectivity
✅ File I/O in sandbox
✅ BYOK inference path

Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw)

Overall PASS,description:Posting smoke test results comment

🔑 BYOK (AOAI api-key) report filed by Smoke Copilot BYOK AOAI (api-key)

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: GitHub Actions Services Connectivity

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

Overall: FAIL

Services are not reachable from the runner. host.docker.internal resolves to 172.17.0.1 but both ports 6379 and 5432 time out — service containers may not be configured in this workflow run.

🔌 Service connectivity validated by Smoke Services

@lpcox
lpcox merged commit 44864f2 into main Jun 21, 2026
85 of 88 checks passed
@lpcox
lpcox deleted the copilot/refactor-split-docker-manager-cleanup branch June 21, 2026 17:50
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.

3 participants