fix(workflow): make _log_uncommitted_code safe in non-git CWDs

[Misty_Cirque (Cyber-Marty)]

Summary

MLflowRecorder._log_uncommitted_code (qlib/workflow/recorder.py:362) ran its git commands via subprocess.check_output(cmd, shell=True) with no stderr capture. In a non-git working directory (containers, CI sandboxes, /tmp, any non-repo checkout), every R.start():

• leaked a multi-line usage: git diff --no-index ... banner straight to the caller's stderr, bypassing qlib's logger entirely — corrupting structured-stderr consumers (JSONL loggers, CI parsers, anything piping output through jq);
• logged three misleading INFO - "Fail to log the uncommitted code of ..." records for what is a perfectly legitimate environment.

The git repo in CWD is not load-bearing for experiment correctness — the feature is an optional reproducibility hook, so it should degrade quietly when its environment isn't available.

Fix

1. Pre-probe the environment: run git rev-parse --is-inside-work-tree once; on failure, skip at DEBUG level instead of running three doomed commands per R.start().
2. Drop shell=True: pass git commands as argument lists (["git", "diff"], ...). The commands use no shell features, and shell=True was a needless injection footgun.
3. Capture stderr on every call (stdout=PIPE, stderr=PIPE), so git diagnostics can never bypass the logger.

Behavior inside a real git work tree is unchanged: the three artifacts (code_diff.txt / code_status.txt / code_cached.txt) are still logged to the recorder.

Tests

New tests/workflow_tests/test_log_uncommitted_code.py:

• non-git CWD: no stderr output, no INFO records (skip logs at DEBUG), no exception
• git work tree: the three diff artifacts are still produced (behavior preserved, verified against the qlib repo checkout itself)

All new and existing workflow tests pass locally (tests/test_workflow.py + both new files, 7 passed).

Fixes #2252 (secondary defect). The primary defect — FileLock path built CWD-relative — is fixed in PR #2337.

[Parafee41 (koriyoshi2041)]

The ordinary non-repo and worktree cases pass locally at e2c4cb26, but the probe has one uncovered Git state: in a freshly initialized bare repository, git rev-parse --is-inside-work-tree exits 0 and prints false. The current implementation then runs all three artifact commands; in a minimal bare repo I observed two INFO failures and one logged artifact, so the advertised silent skip does not hold.

Could the predicate also require result.stdout.strip() == b"true" and add a bare-repository regression beside the existing non-git case? That keeps the change bounded to detecting an actual work tree.

[Misty_Cirque (Cyber-Marty)]

Good catch — thank you for testing this locally and for the precise failure description. You're right: --is-inside-work-tree exits 0 in a bare repository while printing false, so the exit-code-only probe let bare repos through and the artifact commands ran anyway (which also explains the misfiled artifact you observed — git diff in a bare repo exits 0 with empty output, so code_diff.txt got logged with no content while the other two failed).

Fixed in 6fcbb2d:

• the probe now requires stdout.strip() == b"true" in addition to the zero exit, so both the non-repo and bare-repo cases skip at DEBUG level;
• added test_bare_repo_is_silent (git init --bare → assert no stderr, no INFO records, and no logged artifacts) alongside the existing non-git case;
• verified the new test is red against the previous probe implementation and green after the fix; full tests/workflow_tests passes (3 passed).

Thanks again — this made the fix genuinely correct rather than just quiet.

[Parafee41 (koriyoshi2041)]

Verified the changed head at 6fcbb2dd: the probe now requires stdout true, the new bare-repository regression covers the exact missed state, and the focused workflow tests pass 3/3 locally. This resolves my review finding.