Follow-up from the review of #389, which added conductor status --json. Deferred there deliberately: the fix is on the launch path, not in status.
The problem
write_pid_file accepts run_id and log_file (cli/pid.py:99-100) and writes both into the PID file (:123-124). The only production caller never passes either:
# cli/bg_runner.py:394
write_pid_file(proc.pid, web_port, pid_workflow_ref)
Both default to "", so every PID file on disk records empty strings. Confirmed against a real --web-bg launch:
{ "pid": 72319, "port": 53941, "workflow": "e2e-sleeper.yaml",
"run_id": "", "log_file": "" }while /api/info on that same run reported "run_id": "fc3c2375".
Why it matters
conductor status --json advertises both fields, so automation will branch on them and take the else path every time. The run_id is the join key between a background run and its *.events.jsonl, which is the forensic path AGENTS.md documents for debugging --web-bg failures — so the one field that correlates a run to its logs is the one that is blank.
A field that is always empty is worse than an absent one: nothing fails, so nobody notices until someone wires up the writer.
e.get("run_id", "") in the JSON payload also collapses three distinct states into one value — key absent, value empty, and no run id exists.
Suggested fix
run_id, stderr_path, and stdout_path are all in scope at the call site (bg_runner.py:460), so threading them through _finalize_background_launch (:351) into write_pid_file is mechanical.
One wrinkle on log_file: its docstring says "Path to the JSONL event log file", which the parent cannot know — the child's EventLogSubscriber derives it. So either repurpose the field to the bg stderr/stdout logs the parent does have, or rename it to say what it actually holds.
#383 already carries part of this (it passes run_id at the writer for its identity check), so worth coordinating on ordering.
A tests/test_cli/test_bg_runner.py test asserting the launcher records a non-empty run_id would stop it silently regressing.
Follow-up from the review of #389, which added
conductor status --json. Deferred there deliberately: the fix is on the launch path, not instatus.The problem
write_pid_fileacceptsrun_idandlog_file(cli/pid.py:99-100) and writes both into the PID file (:123-124). The only production caller never passes either:Both default to
"", so every PID file on disk records empty strings. Confirmed against a real--web-bglaunch:{ "pid": 72319, "port": 53941, "workflow": "e2e-sleeper.yaml", "run_id": "", "log_file": "" }while
/api/infoon that same run reported"run_id": "fc3c2375".Why it matters
conductor status --jsonadvertises both fields, so automation will branch on them and take the else path every time. Therun_idis the join key between a background run and its*.events.jsonl, which is the forensic pathAGENTS.mddocuments for debugging--web-bgfailures — so the one field that correlates a run to its logs is the one that is blank.A field that is always empty is worse than an absent one: nothing fails, so nobody notices until someone wires up the writer.
e.get("run_id", "")in the JSON payload also collapses three distinct states into one value — key absent, value empty, and no run id exists.Suggested fix
run_id,stderr_path, andstdout_pathare all in scope at the call site (bg_runner.py:460), so threading them through_finalize_background_launch(:351) intowrite_pid_fileis mechanical.One wrinkle on
log_file: its docstring says "Path to the JSONL event log file", which the parent cannot know — the child'sEventLogSubscriberderives it. So either repurpose the field to the bg stderr/stdout logs the parent does have, or rename it to say what it actually holds.#383 already carries part of this (it passes
run_idat the writer for its identity check), so worth coordinating on ordering.A
tests/test_cli/test_bg_runner.pytest asserting the launcher records a non-emptyrun_idwould stop it silently regressing.