fix: parquet fallback, non-identifier env keys, umask scoping (PR #323 follow-ups)

[xdotli]

Fixes three infrastructure bugs that review bots flagged on merged PR #323 but were never addressed. Each is a separate commit with a regression test whose docstring names PR #323 (per AGENTS.md).

Bug G [Codex P1] — parquet conversion outside the try, no fallback

src/benchflow/traces/huggingface.py _download_hf_dataset — the split-aware download loop caught exceptions from hf_hub_download, but _parquet_to_jsonl(...) was outside that try. If a parquet file downloaded but pyarrow was missing (or parquet decoding failed), the exception propagated immediately and the JSONL candidates / _download_via_api fallback were never tried — a regression from the prior behavior that alternated formats.

Fix: move the parquet conversion inside the per-candidate try, so a conversion failure falls through to the JSONL candidates and then the API fallback.

Bug H [Codex P2] — env keys that aren't valid shell identifiers

src/benchflow/sandbox/docker.py _wrap_command_with_env_file — env was serialized as shell export {k}=... lines and sourced. Keys that are valid process env names but not valid POSIX shell identifiers (containing . or -) made . {env_path} return non-zero, so the user command never ran. The old docker compose exec -e KEY=VALUE path had no such constraint.

Fix: only emit export lines for keys matching the shell-identifier grammar; skip non-identifier keys with a warning. Valid keys still flow through. No ps aux leak is reintroduced — the base64 env-file mechanism is unchanged.

Bug I [Cursor Medium] — umask 077 leaks into the user command

Same function — umask 077 (set to protect the temp env file) ran at the top of the bash -c chain and persisted into the user's actual command, so files the command created got mode-0600 unexpectedly.

Fix: scope umask 077 to a subshell (umask 077 && ...) around just the env-file write, restoring the default umask before the user command runs.

Tests

• test_parquet_conversion_failure_falls_through_to_jsonl (Bug G)
• test_umask_scoped_to_env_file_write (Bug I)
• test_non_identifier_env_keys_do_not_break_exec (Bug H)

All three verified to fail against unfixed source and pass with the fixes.

CI

ruff format --check, ruff check ., ty check src/, and pytest tests/ (1313 passed, 27 skipped) all pass locally.

Source: unaddressed review-bot comments on PR #323.

---

Note

Medium Risk
Medium risk because it changes how DockerSandbox.exec materializes environment variables and alters HuggingFace dataset download fallback behavior; regressions would impact command execution semantics and dataset ingestion paths.

Overview
Fixes two regression-prone infrastructure paths.

DockerSandbox._wrap_command_with_env_file now scopes umask 077 to a subshell so it doesn’t affect the user command, and skips non-POSIX-identifier env keys (logging a warning) so sourcing the env file can’t abort execution.

HuggingFace dataset download now keeps parquet-to-JSONL conversion inside the per-candidate try/except, ensuring conversion failures (e.g., missing pyarrow) fall through to JSONL/API fallbacks. Adds targeted regression tests for all three scenarios.

^{Reviewed by Cursor Bugbot for commit d18cd80. Bugbot is set up for automated code reviews on this repo. Configure here.}

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.