Skip to content

chore(runtime): warn on cleanup failures in shell and web_fetch_to_file#6255

Merged
houko merged 2 commits into
librefang:mainfrom
maoxin1234:fix-low-priority-bugs-shell-webfetch
Jun 21, 2026
Merged

chore(runtime): warn on cleanup failures in shell and web_fetch_to_file#6255
houko merged 2 commits into
librefang:mainfrom
maoxin1234:fix-low-priority-bugs-shell-webfetch

Conversation

@maoxin1234

Copy link
Copy Markdown
Contributor

Summary

Replaces two silent let _ = ignores with tracing::warn logs so cleanup failures are observable:

  1. tool_runner/shell.rs: when shell_exec is interrupted or times out, it now logs a warning if kill_process_tree fails. Previously the failure was swallowed, making it hard to notice orphaned processes.

  2. web_fetch_to_file.rs: when the atomic write or rename fails, it now logs a warning if removing the temporary partial-download file fails. Previously the temp file leak was silent.

Verification

  • cargo fmt --all
  • cargo clippy -p librefang-runtime --all-targets -- -D warnings
  • cargo test -p librefang-runtime web_fetch_to_file (4 passed)
  • cargo test -p librefang-runtime shell (108 passed)

Replace silent `let _ =` ignores with `tracing::warn` logs when:
- `shell_exec` fails to kill the process tree on interrupt or timeout.
- `web_fetch_to_file` fails to remove the temporary partial-download file
  after a write or rename error.

This makes orphaned processes / leaked temp files observable without
changing the public error returned to callers.
@github-actions github-actions Bot added area/runtime Agent loop, LLM drivers, WASM sandbox size/S 10-49 lines changed labels Jun 20, 2026

@houko houko 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.

LGTM. Reviewed: replaces three silent let _ = cleanup ignores with tracing::warn structured logs (orphaned process trees, leaked partial-download temp files become observable); control flow is preserved exactly and it follows the repo logging conventions. Real CI — Quality/clippy, all 4 Ubuntu test shards, Unit, Build, Security, Live Integration Smoke — is green.

The red detect-secrets scan check is a stale-base artifact: this branch predates #6262, which retired detect-secrets in favor of gitleaks, so the legacy baseline gate fails here. It is not a real secret (the change is logging-only). Merging onto current main — which uses gitleaks — is clean.

@houko houko 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.

LGTM — reviewed clean (structured-log cleanup, control flow preserved); real Rust CI green; branch updated to current main.

@houko
houko enabled auto-merge (squash) June 21, 2026 15:56
@houko
houko merged commit dfb53e0 into librefang:main Jun 21, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/runtime Agent loop, LLM drivers, WASM sandbox size/S 10-49 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants