Skip to content

Fix isolation error wording and log-level rationale from #5794 review#5853

Merged
rdimitrov merged 1 commit into
mainfrom
pr-5794-feedback
Jul 20, 2026
Merged

Fix isolation error wording and log-level rationale from #5794 review#5853
rdimitrov merged 1 commit into
mainfrom
pr-5794-feedback

Conversation

@aponcedeleonch

Copy link
Copy Markdown
Member

Summary

#5794 (merged) reconciled network isolation with network mode. rdimitrov's
post-merge review left two follow-up comments this PR addresses:

  • The fail-fast error for --isolate-network=true conflicting with a
    non-bridge network mode assumed the mode always came from --network,
    telling users to drop --network host even when the mode actually came
    from a --permission-profile file (where there's no --network flag to
    drop). The error now names the resolved mode directly instead of assuming
    a specific flag.
  • Clarified in degradeNetworkIsolation why the none and host/custom
    degrade paths intentionally log at different levels: none is DEBUG
    (redundant, not a security reduction), while host/custom stays at WARN
    even on read paths (thv list, status, export) since it means the
    user's --isolate-network request is being silently ignored.

Type of change

  • Bug fix

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix)

Updated TestRunConfigBuilder_NetworkIsolationReconciliation in
pkg/runner/config_builder_test.go to assert the error names the resolved
mode instead of an assumed --network flag.

Does this introduce a user-facing change?

Yes. The fail-fast error message when --isolate-network=true conflicts
with a non-bridge network mode now names the resolved network mode
directly (e.g. "host") instead of assuming it came from --network.

🤖 Generated with Claude Code

Address post-merge review feedback on #5794: the fail-fast error for
--isolate-network conflicting with a non-bridge network mode assumed
the mode came from --network, but it can also come from a
--permission-profile file, making the suggested "drop --network X"
remedy inactionable. The error now names the resolved mode directly.

Also clarify why the "none" and host/custom degrade paths log at
different levels in ReadJSON.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the size/XS Extra small PR: < 100 lines changed label Jul 17, 2026
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.23%. Comparing base (e14bb31) to head (3bcf1e8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5853      +/-   ##
==========================================
+ Coverage   71.16%   71.23%   +0.06%     
==========================================
  Files         689      689              
  Lines       70110    70110              
==========================================
+ Hits        49896    49941      +45     
+ Misses      16595    16533      -62     
- Partials     3619     3636      +17     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rdimitrov
rdimitrov merged commit ee9612b into main Jul 20, 2026
107 of 110 checks passed
@rdimitrov
rdimitrov deleted the pr-5794-feedback branch July 20, 2026 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants