feat(cli): add runtime.log_file YAML default for --log-file

[nskun (nskun)]

Summary

Add a log_file field to RuntimeConfig so workflow authors can set a default log destination in YAML.
The CLI --log-file flag continues to take precedence when specified.

Motivation

Currently, file logging requires passing --log-file on every invocation.
When forgotten, crashes leave no diagnostic log.
This is especially painful for long-running workflows(30-60 min).
Other runtime fields(default_model, max_tokens, temperature, timeout) already support YAML defaults -- log_file should follow the same pattern.

Usage

workflow:
runtime:
log_file: auto

Accepts the same values as CLI --log-file:
・auto -- Generate a timestamped path in $TMPDIR/conductor/.
・An explicit path (e.g. ./logs/run.log) -- Write to that path directly.
・Omitted -- No file logging(current default, unchanged).

Design

CLI --log-file and YAML runtime.log_file become available at different times.
CLI is known before YAML parse: YAML is known only after load_config(). So:
・CLI --log-file specified -> log init before YAML parse(existing behavior, unchanged)
・CLI unset -> after load_config(), check runtime.log_file and init if present.
・CLI takes precedence when both are set.

Closes#472

[Jason Robert (jrob5756)]

Took a careful read through this. The gap you're closing is real, and a few things here are done better than average: the run/resume parity is genuine rather than just claimed, the new warning correctly passes the path and str(e) through styled() as values instead of into the template, and you updated the bundled skill references alongside the user docs, which people usually forget.

A few things I'd want sorted before it merges.

CI is red right now. ruff format --check wants the new warning block collapsed onto one line, and it's the only formatting deviation in the tree. make format handles it.

The one I'd fix first is that Path(configured) never calls .expanduser(). Writing log_file: ~/logs/run.log in YAML is a natural thing to do, and because init_file_logging runs mkdir(parents=True) it succeeds: you get a directory named literally ~ under the process CWD, the log inside it, and no warning, because nothing raised. The CLI flag dodges this only because the shell expands ~ before Python sees it. YAML has no shell, so this field is what makes the case reachable. Every other user-supplied path in the tree expands it, runtime.working_dir included.

Close behind: generate_log_path() sits outside the try/except, and its mkdir(parents=True, exist_ok=True) still raises if $TMPDIR/conductor happens to be a file, or on a read-only or full runner. So log_file: auto can take down a workflow that would otherwise run fine, with an error panel naming neither log_file nor the workflow. Right now an explicit bad path warns and continues while auto aborts, and those should match.

Worth deciding separately: init_file_logging opens with mode "w". That was fine when the only way in was typing --log-file at the prompt. As a persistent YAML default it means conductor resume truncates the crash log before writing to it, which is the one moment you most want that file intact. auto is safe because of the random suffix; explicit paths aren't, and no doc mentions it.

Finally, log_file: auto is the example value in all four doc surfaces you added and no test exercises it on either path. Deleting the if/else and keeping only Path(configured) leaves the suite green.

Rest is inline. None of it is about the idea, which I think is worth having. There's also no CHANGELOG.md entry under [Unreleased], which recent feature PRs have all added.

[Jason Robert (jrob5756)]

ruff format wants this on one line now that it's out of the nested if and fits in 100 chars. It's the only formatting deviation in the tree, and .github/workflows/ci.yml:61 runs ruff format --check, so the lint job is failing on this alone.

Suggested change

	_verbose_console.print(
	styled(
	"[bold yellow]Warning:[/bold yellow] Cannot open log file {}: {}", log_path, e
	styled("[bold yellow]Warning:[/bold yellow] Cannot open log file {}: {}", log_path, e)

[Jason Robert (jrob5756)]

Four things land in this block.

generate_log_path() is outside the guard. Its mkdir(parents=True, exist_ok=True) still raises FileExistsError when $TMPDIR/conductor is a file, and PermissionError or ENOSPC on a constrained runner. A logging setting shouldn't be able to abort the run, and it's inconsistent with the explicit-path branch one line below, which warns and continues.

Path(configured) doesn't expand ~. Since init_file_logging creates parents, log_file: ~/logs/run.log quietly writes into a directory named ~ under the CWD and reports success. The shell saves the CLI flag from this; YAML doesn't.

A NUL byte in the value raises ValueError, which except OSError doesn't catch, so it escapes as an unhandled traceback.

And the bool from _try_init_file_logging is dropped here, so log_file keeps naming a file that was never opened. Nothing reads it wrongly today, but only because the _file_console is not None check at line 2313 happens to cover for it.

Suggested change

	ifconfiguredisnotNone:
	ifconfigured.lower() =="auto":
	log_file=generate_log_path(workflow_path.stem)
	else:
	log_file=Path(configured)
	_try_init_file_logging(log_file)
	ifconfiguredisnotNone:
	try:
	ifconfigured.strip().lower() =="auto":
	log_file=generate_log_path(workflow_path.stem)
	else:
	log_file=Path(configured).expanduser()
	except (OSError, ValueError) ase:
	_verbose_console.print(
	styled(
	"[bold yellow]Warning:[/bold yellow] Cannot prepare log file {}: {}",
	configured,
	e,
	)
	)
	log_file=None
	iflog_fileisnotNoneandnot_try_init_file_logging(log_file):
	log_file=None

[Jason Robert (jrob5756)]

Same four issues as the run side. Given these two blocks are identical apart from the path variable, and AGENTS.md names drift between run_workflow_async and resume_workflow_async as a hazard, I'd lift this into one helper rather than fix it twice.

A shared resolve_log_file(value, stem) next to generate_log_path would also absorb the two copies in cli/app.py (:663 and :1157). Those have already drifted: app.py:1158 falls back to a "resume" stem where this line uses the checkpoint-derived one. Four copies of the auto vocabulary means the expanduser() and strip() fixes have to be applied four times to hold.

One more thing specific to resume. Because the YAML value is read after the checkpoint block, this log misses Loading checkpoint, the workflow-hash-changed warning, and Resuming from agent, all of which --log-file captures. Two spellings of the same setting produce different logs, and the docs present them as equivalent.

[Jason Robert (jrob5756)]

A couple of invariants the type could carry instead of the call site.

An empty or whitespace-only value clears the is not None guard downstream and becomes Path("."), which fails as IsADirectoryError and prints Cannot open log file .: with an errno naming ''. Neither half points back at this field. SkillDiscoveryConfig.validate_exclude a few lines below rejects blank entries for the same reason, and session_key at line 1532 already uses the constraint below.

Stripping also fixes " auto ", which currently becomes a file named ␣auto␣ rather than matching the sentinel.

The docstring is worth two additions as well: auto is matched case-insensitively, and only the top-level workflow's value is read. A sub-workflow setting this gets no log, no warning and no error, which is surprising when working_dir, skills and default_model all apply per sub-workflow. runtime.checkpoint has the same root-only scope and says so.

Suggested change

	log_file: str|None=None
	"""Default log file path, or ``auto`` to generate one automatically."""
	log_file: Annotated[str, StringConstraints(strip_whitespace=True, min_length=1)] |None=None
	"""Defaultlogfilepath, or``auto`` (case-insensitive) togenerateoneautomatically.
	Honouredonlyforthetop-levelworkflow; ignoredina``type: workflow``sub-workflow.
	"""

[Jason Robert (jrob5756)]

_write_workflow always interpolates an explicit path, so configured.lower() == "auto" is never taken on run or resume. That's the value in every doc example this PR adds, and it's the branch with the most room to go wrong: dropping the if/else, or just the .lower(), leaves the whole suite green.

tests/conftest.py already redirects tempfile.gettempdir() to tmp_path for every test and TestGenerateLogPath relies on it, so this is a small addition. Parameterising the helper to take the raw value would let one test assert a single *.log lands under gettempdir()/conductor/ with the workflow stem in its name.

While you're here, both integration tests assert only log_path.exists(). Mode "w" creates the file at init regardless, so neither would notice if no log line ever reached it. test_try_init_with_valid_path gets this right by reading the content back.

[Jason Robert (jrob5756)]

This patches in a plain rich.Console, but production uses _SilentAwareConsole, whose print is gated on is_verbose(). The assertion therefore passes against a console that never suppresses anything.

Under --silent the real warning is dropped and the user gets nothing at all: no file, no message, exit 0, and nothing in the JSONL event log either. So this test currently vouches for feedback that doesn't exist in the configuration the docs actually recommend, since execution.md suggests pairing file logging with --silent.

Patching with _SilentAwareConsole and asserting both verbosity states would catch it. Separately, I'd argue a diagnostics subsystem failing to start isn't progress output and shouldn't be silenced by --silent at all; routing it to console or emitting it as a workflow event would make it durable.

[nskun (nskun)]

Thank you so much for the incredibly thorough review — I really appreciate the time and care you put into it.
I’ve read through all the feedback, and it’s extremely helpful.
It may take me a couple of days to work through everything and push an update, but I’ll ping you once it’s ready for another look.

[Jason Robert (jrob5756)]

Good progress since the last pass! I started to review but didn't realize you were working on it still. My apologies for that. Please let me know when its ready for another pass. Cheers!