fix(cli): make conductor stop confirm the process actually stopped

[Frank Li (franklixuefei)]

Closes#344.

The bug

conductor stop sent one best-effort signal, printed Stopped, and deleted the PID file unconditionally. On Windows that signal reliably fails — as Jason Robert (@jrob5756) diagnosed on the issue, GenerateConsoleCtrlEvent needs a shared console, which a separate stop invocation does not have. So the usual outcome was: workflow still running, user told it stopped, PID file gone. The run became an untracked orphan still burning tokens, and because stop had deregistered it there was no supported way to find it again. In my case relaunching produced two concurrent runs racing on the same worktree.

That correction also killed my original theory (I had guessed a missing CREATE_NEW_PROCESS_GROUP; the bg child already gets it at bg_runner.py:48,79,151). Following it up surfaced something worse and cross-platform: the bg child runs --no-interactive, so no SIGTERM handler is installed anywhere. stop is therefore a hard kill with no checkpoint on every platform, not just Windows.

The change

A ladder, with a bounded wait after each rung, and no rung trusted to have worked.

1. POST /api/kill — the only rung that lets the run write a resume checkpoint, so it is always tried first.
2. The platform signal — best-effort; genuinely works on POSIX, usually not on Windows.
3. Forceful termination.

The central rule: a PID file is removed only once its process is confirmed dead. A survivor keeps its registration and the command exits 2, so a run can never again be silently deregistered while alive.

Two details that matter more than they look:

• HTTP 200 from /api/kill is an acknowledgement, not a death certificate. It sets an asyncio event and returns; run.py:1441's asyncio.gather drain is unbounded, so a provider that swallows cancellation leaves the child alive indefinitely after the endpoint already returned success. Every rung is followed by a real liveness check.
• Forceful termination is gated on identity. Terminating by PID with no identity check would be strictly worse than the bug being fixed: today's broken signal kills nothing, whereas TerminateProcess on a recycled PID kills a bystander. PID files outlive crashes and reboots, and Windows recycles PIDs aggressively.

Identity is by PID, not run id

My first attempt compared run_id, which already existed in the PID-file schema and in /api/info but was never populated by the launcher. Wiring it up was not enough — it cannot work for resume --web-bg, and a post-implementation review caught it before this PR went up.

The launcher generates a fresh run id for the PID file, but a resumed child reuses the checkpoint's run id: event_log.py:104-116 returns early on existing_run_id before ever consulting CONDUCTOR_RUN_ID. So /api/info reported an id that could never match, and every resumed run looked like somebody else's process — graceful rung skipped, a misleading "a different run took over this port" warning, and on Windows the forceful rung refused. POSIX masked it entirely, because SIGTERM terminates the child before the gate is ever reached.

/api/info now reports os.getpid() unconditionally. The dashboard runs in the same process as the workflow, so this is direct proof of ownership, and it fixes three things at once:

	run-id comparison	pid comparison
run --web-bg	works	works
resume --web-bg	always mismatches	works
killed during startup ({} until workflow_started)	unconfirmable	works
PID file from an older conductor (no run id)	unconfirmable	works

Run-id comparison is kept as a fallback so a dashboard predating this change is still identifiable.

Identity is a tristate, and that distinction is load-bearing

• MISMATCHED — positive evidence this PID belongs to someone else → blocks every PID-directed action, including the polite signal. Refusing only the forceful rung would have been incoherent: on POSIX the polite rung is SIGTERM, perfectly capable of killing the bystander the gate exists to protect.
• UNCONFIRMED — no evidence either way → still signals, because that is all the previous implementation ever did. Refusing here would regress legacy PID files on POSIX, where the signal works.
• CONFIRMED — full ladder.

Identity is re-confirmed immediately before the irreversible rung, since up to ten seconds of waiting elapse after the first check and the PID could be recycled in that window. This narrows the race rather than closing it; the fully general fix is to retain a process handle across the whole ladder (Windows) or a pidfd (Linux), which I have deliberately kept out of this PR — happy to follow up.

Everything else

• Tristate Liveness internally, so "the probe failed" can no longer be mistaken for "confirmed running". The bool wrappers are retained, so the conductor stop crashes on Windows: OSError [WinError 11] from os.kill(pid, 0) in _is_process_alive #166 listing behaviour is byte-for-byte unchanged — importantly, UNKNOWN still keeps the PID file there, which test_unexpected_oserror_does_not_propagate pins.
• terminate_process returning UNKNOWN reports unconfirmed, not survived. A failed probe is not evidence the process lived, and JSON consumers would read survived as "definitely still alive".
• Removal is by file path with a re-read identity check, replacing remove_pid_file(port). Port matching could unlink a newer run's registration if that run bound the same port mid-stop — and making the unlink conditional on the old run's death makes that race more likely to fire, not less.
• Windows forceful path opens once with PROCESS_TERMINATE | SYNCHRONIZE and both terminates and waits on the same handle. An open handle pins the PID, so the confirmation cannot be fooled by recycling mid-call. _TERMINATION_EXIT_CODE = 1 != STILL_ACTIVE, so a later probe reads DEAD.
• --json, emitting pid/port/run_id/outcome/rung per run — the issue's own evidence is that automation trusted stop's prose and double-launched. An exit code says something failed; it does not say which run survived. Prose goes to console (stderr) and JSON to output_console (stdout), so they cannot corrupt each other.
• --force for the unidentifiable case.

Breaking changes

Exit codes. Previously every path except an unknown --port returned 0.

code	meaning
0	all targets confirmed stopped, or already gone, or nothing was running
1	--port matched nothing, or the target was ambiguous (multiple runs, no --port/--all)
2	at least one target survived or could not be confirmed stopped

CI teardowns running conductor stop --all under set -e will now fail when a run genuinely survives — which is the point, but it is a behaviour change. "Nothing running" and "already exited" are deliberately 0. The ambiguous case moved from 0 to 1 because it stops nothing, and reporting success for that is exactly the class of bug this PR is about.

Pre-existing PID files (written before this change, no run id) are identified by PID as soon as their dashboard is reachable, so in practice they behave normally. If the dashboard is unreachable and the process survives both non-forceful rungs, stop now refuses to force-terminate and says so, instead of silently deregistering a live run.

Tests

tests/test_cli/test_stop_ladder.py covers the ladder, the identity tristate, and the removal rules; tests/test_web/test_server.py covers /api/info reporting pid before any workflow event.

I verified the guards actually bite by simulating three separate reverts — each fails, and the fixed tree passes:

simulated revert	result
unlink unconditionally	2 failed
remove the identity gate	2 failed
identity by run id only (the resume bug)	2 failed
fixed tree	31 passed

Four pre-existing tests in test_stop.py asserted the old contract — that stop reports success merely because os.kill did not raise — and were rewritten to assert the new one. I want to flag that explicitly rather than bury it: those tests encoded the bug.

One of my own rewrites was itself broken, and I would rather say so than have it found in review: test_pid_file_is_removed_when_process_confirmed_gone patched conductor.cli.app.os.kill, which mutates the shared os module object — on POSIX that also patches pid.py's liveness probe, turning DEAD into UNKNOWN. It passed on Windows and would have failed on the Ubuntu CI runners. It now patches process_liveness directly. The --all survivor test had a similar latent flake, keying outcomes off call order when target order comes from Path.glob, whose ordering is filesystem-dependent; it is now keyed off the pid.

Full tests/test_cli + tests/test_web run is an exact failure-set match against clean main — zero new failures. (My sandbox has no network access to PyPI, so pytest-asyncio is unavailable and ~48 async tests error identically before and after; I diffed the failure sets rather than trusting the count.) ruff check and ruff format --check are clean.

Not in this PR

• Retaining a process handle / pidfd across the whole ladder to fully close the identity→terminate race described above.
• A SIGTERM handler for the bg child, so stop can checkpoint on POSIX. Note listener.py:244-250 chains to _previous_sigterm, which is normally signal.SIG_DFL — not callable — so the existing handler silently swallows SIGTERM, contradicting its own docstring. That wants its own fix.
• A per-command deadline for --all. Each target is bounded (~22s worst case) but N targets are sequential with no overall cap.
• PYTHONUTF8=1 in _build_bg_env — deliberately excluded. pid.py reads and writing PID files with no explicit encoding, so making the child UTF-8 while stop stays on the locale codec would split-brain them, and UnicodeDecodeError is a ValueError sibling that the existing except (json.JSONDecodeError, OSError) would not catch.

Related but independent, from the same run: #342 / #381 (UnicodeEncodeError in JSON output) and #382 (MarkupError from unescaped agent text). Three separate boundaries, three separate fixes.

[Codecov Comments Bot (codecov-commenter)]

Codecov Report

❌ Patch coverage is 96.88889% with 7 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@e6b5671). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/conductor/cli/pid.py	93.51%	7 Missing ⚠️

Additional details and impacted files

@@ Coverage Diff @@## main #383 +/- ##
=======================================
Coverage ? 91.71% =======================================
Files ? 109 Lines ? 17806 Branches ? 0 =======================================
Hits ? 16330 Misses ? 1476 Partials ? 0 

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Jason Robert (jrob5756)]

Reviewed the ladder, the identity gate, the pid primitives, and the tests. The approach is right, and the Windows termination path that keeps one handle open for both the terminate and the wait is the correct way to do it. Most of my notes are small. One I think should be fixed first.

The blocker is the MISMATCHED gate. With --force, a PID that was positively identified as belonging to a different process still gets signalled and force-terminated. The command prints "Refusing to act on it" and then "Stopped" on the next line, returns outcome stopped, removes the PID file, and exits 0. I reproduced it locally. That is the same shape as the bug this PR fixes, so it seems worth closing before this lands.

Two things I could not attach to a diff line:

write_pid_file (pid.py:167) uses write_text, which truncates and then streams, while read_pid_files (pid.py:189) unlinks anything it cannot parse and says nothing. A stop that reads a PID file during that write window gets a JSONDecodeError and silently deregisters a live run, which is the orphaning failure arriving through the read side. Writing to a temp file and calling replace(), plus a warning before the unlink, would close it.

--force still cannot clear an entry whose liveness probe keeps failing, which was the #166 escape hatch we discussed on the issue. I checked: stop --port N --force leaves the file and exits 2. A stuck entry therefore makes bare stop permanently ambiguous and stop --all permanently exit 2, so a CI teardown never recovers. Letting --force unlink on an unconfirmed outcome, with a loud warning, would restore it.

The writeup on the PR is genuinely useful, particularly calling out your own broken test rewrite and the four old tests that encoded the bug. Thanks again!

[Jason Robert (jrob5756)]

The Identity docstring says MISMATCHED "must block every PID-directed action, not just the forceful one", but and not force lets it through. With --force against a mismatched dashboard I get:

Warning: the dashboard on port 8080 is PID 9999, but the PID file records 4242. Refusing to act on it.
Stopped workflow 'w' (PID 4242, port 8080)

SIGTERM went to 4242, the outcome came back stopped, the PID file was removed, and the exit code was 0. So we signal a process we just identified as someone else's, tell the user we refused, then tell them it stopped. The real run keeps going, and automation reading that exit code concludes it is safe to start another.

The --force help says "even when the run's identity cannot be confirmed", which reads as UNCONFIRMED only. I would keep that reading and let a positive mismatch refuse unconditionally.

Suggested change

	ifidentityisIdentity.MISMATCHEDandnotforce:
	ifidentityisIdentity.MISMATCHED:

There is also no test for MISMATCHED with force=True. test_force_overrides_the_identity_gate patches UNCONFIRMED, and test_positive_mismatch_signals_nothing_at_all only covers the default.

[Jason Robert (jrob5756)]

Whichever way the MISMATCHED question lands, this wording should match it. "cannot be confirmed" describes UNCONFIRMED, and right now --force also overrides a positive mismatch.

Assuming the gate above is fixed:

Suggested change

	"Force-terminate even when the run's identity cannot be confirmed. "
	"Dangerous: the recorded PID may have been recycled onto another process."
	"Force-terminate even when the run's identity cannot be confirmed. "
	"Dangerous: the recorded PID may have been recycled onto another process. "
	"Does not override a confirmed identity mismatch."

[Jason Robert (jrob5756)]

Nothing under src/conductor/ configures logging, so logger.debug never reaches a user. Connection refused is the expected case and belongs at debug, but a 5xx from the dashboard, a proxy or TLS failure, and a read timeout all vanish here too, and those are things someone can act on. Before this PR the user at least got an OSError traceback to paste into a bug report.

Suggested change

	exceptExceptionasexc: # noqa: BLE001 - any failure means "cannot confirm"
	logger.debug("Identity probe on port %s failed: %s", port, exc)
	returnIdentity.UNCONFIRMED
	except (httpx.ConnectError, httpx.ConnectTimeout):
	logger.debug("Identity probe on port %s: dashboard unreachable", port)
	returnIdentity.UNCONFIRMED
	exceptExceptionasexc: # noqa: BLE001 - any failure means "cannot confirm"
	logger.warning(
	"Identity probe on port %s failed (%s: %s); the graceful rung will be skipped.",
	port,
	type(exc).__name__,
	exc,
	)
	returnIdentity.UNCONFIRMED

_request_graceful_kill and _signal_process have the same problem.

[Jason Robert (jrob5756)]

I checked this against main: _finalize_background_launch called write_pid_file(proc.pid, web_port, pid_workflow_ref) with no run_id, and there was no other production caller. Every PID file written by a released version therefore has run_id == "".

So a dashboard old enough not to report pid always comes with a PID file that has no run id, and this returns UNCONFIRMED at line 1327. The CONFIRMED and MISMATCHED returns below can only fire from a hand-edited file or a third party answering on the port.

Harmless, but the PR description offers this as the thing that keeps an older dashboard identifiable, and it cannot do that. Either drop the fallback and return UNCONFIRMED, or reword the rationale so the next reader does not rely on it.

[Jason Robert (jrob5756)]

Liveness and Identity are enums compared with is. outcome is a bare string compared with in (...), and the same success tuple is repeated at 1251, 1252, and 1256. It is also the public --json contract, so a typo at any one site silently misfiles a survivor as stopped.

StrEnum serialises through json.dumps as the plain string and compares equal to str, so an Outcome enum with an is_success property costs nothing in JSON output or in the existing tests, and puts the success rule in one place. Cheap while the file is already open.

[Jason Robert (jrob5756)]

Minor: not (A or B) is not A and not B, and reading it the second way saves unpacking a De Morgan in the one gate that guards an irreversible action.

Suggested change

	ifnot (identityisIdentity.CONFIRMEDorforce):
	ifidentityisnotIdentity.CONFIRMEDandnotforce:

[Jason Robert (jrob5756)]

This function performs no identity check, so the bold "must" is a contract for the caller rather than anything enforced here. Worth stating that plainly so nobody reads it as a guarantee the function provides.

Suggested change

	Thisisthelastrungofthestopladderandtheonlyonethatcannotbe
	ignoredbythetarget. Callers**must**confirmprocessidentitybefore
	invokingit — aPIDreadfromastalefilemaysincehavebeenrecycled
	ontoanunrelatedprocess (seeissue#344).
	Thisisthelastrungofthestopladderandtheonlyonethatcannotbe
	ignoredbythetarget. Itperformsnoidentitycheckitself; thecalleris
	responsibleforconfirmingthePIDstillbelongstotheintendedrun (see
	``conductor.cli.app._confirm_identity``), becauseaPIDreadfromastale
	filemaysincehavebeenrecycledontoanunrelatedprocess (issue#344).

[Jason Robert (jrob5756)]

No production caller is left. stop uses remove_pid_file_at, and run.py uses remove_pid_file_for_current_process. The only references are the three tests in test_pid.py.

"compatibility with any external caller" is carrying a lot of weight for a module inside conductor.cli, when the wheel ships one console script and conductor/__init__.py exports only __version__. I would either delete it along with TestRemovePidFile, or name a removal version so it does not sit here indefinitely.

[Frank Li (franklixuefei)]

Thanks — the MISMATCHED gate was a real bug and you reproduced it exactly. All four items are addressed in 9642c2a.

The blocker: --force overrode a positive mismatch

Confirmed at source before fixing. Three separate places conspired: the rung-2 guard read if identity is Identity.MISMATCHED and not force, rung 3 skipped the re-confirmation entirely under --force, and its refusal was if not (identity is Identity.CONFIRMED or force) — so --force short-circuited all three. Hence "Refusing to act on it" from _confirm_identity, then Stopped from the ladder, outcome stopped, exit 0.

What makes it clearly a bug rather than a judgement call is that the Identity docstring in this same PR already states MISMATCHED "must block every PID-directed action, not just the forceful one." The code simply did not do what the doc said.

The rule is now that sentence: --force overrides uncertainty, never positive evidence that a PID belongs to someone else. UNCONFIRMED still escalates under --force, so the #166 hatch and older run_id-less PID files are untouched.

Rung 3 also re-confirms under --force now. It was skipped exactly where the consequences are worst — seconds of waiting elapse before the irreversible rung, which is the recycle window the re-check exists for.

One deliberate contract change: a mismatch now reports outcome: "mismatched", not "unconfirmed". "Unconfirmed" claims we could not tell; a mismatch is the opposite. I checked both consumers first — each tests against the success allow-list ("stopped", "already-exited") — so a new value is safe and still counts as a failure.

write_pid_file / read_pid_files

Both confirmed. write_text truncates then streams, and every reader treats unparseable JSON as a dead run and unlinks it — so the orphaning arrives through the read side, as you said. Writes now go to a temp file and land via os.replace; the temp name ends in .tmp so the *.pid glob cannot see it, and it is cleaned up if the write fails. read_pid_files now logs before any unlink.

--force and the unprobeable entry (#166)

Restored, and deliberately narrow: it fires only for outcome == "unconfirmed" at rung terminate — the case where the liveness probe itself failed. It does not fire for refused (we chose not to act), mismatched (someone else's PID), or survived — that last one matters, because the process is demonstrably alive and removing its file is precisely the orphaning bug.

One thing I did not do, and want your read on: that invocation still exits 2. Clearing a record is not the same as confirming a stop, so reporting success felt like a lie. The wedge is gone either way — the entry is removed, so the nextstop --all exits 0. If you would rather CI not need || true on the invocation that clears, say so and I will change it.

Tests

Taking your point from #381 seriously here, since nothing previously asserted any of this. test_force_overrides_the_identity_gate uses UNCONFIRMED, so the mismatch case was a gap, not a disagreement — which is why the bug survived.

Each new test was run against the unfixed code:

Test	Without fix
test_force_does_not_override_a_positive_mismatch	❌ fails
test_force_reconfirms_identity_before_the_irreversible_rung	❌ fails
test_write_is_atomic_so_a_reader_never_sees_a_partial_file	❌ fails
test_unreadable_pid_file_logs_before_unlinking	❌ fails
test_pid_file_without_a_pid_logs_before_unlinking	❌ fails
test_force_can_clear_an_entry_whose_liveness_cannot_be_probed	❌ fails
test_temp_file_is_not_left_behind	✅ passes either way

That last row is deliberate: it guards the new write path against leaving strays, it is not a regression test, and I would rather say so than let it look like coverage it is not.

Two existing assertions changed from "unconfirmed" to "mismatched", each with a comment explaining why it is intentional.

Gates

583 passed in tests/test_cli. ruff check unchanged at 4 pre-existing findings (UP042 on three str+Enum classes, one of which this PR never touched); my files are ruff format clean and ty clean — including fixing the os.replace annotation, since you flagged that exact class of ty error on #381. ty check src unchanged at 65.

CHANGELOG entry added under Unreleased → Fixed.

[Jason Robert (jrob5756)]

Re-ran everything against 9642c2a instead of taking the summary on faith. The blocker is fixed, and two of the things you did go past what I asked for.

Verified

Reran my original reproduction. Neither the signal nor the terminate fires now, the outcome comes back mismatched, and the console finally reads straight: a refusal, then another refusal, with no "Stopped" sitting three lines under a warning that says we refused.

Re-confirming identity at rung 3 under --force is better than what I suggested. I only asked you to stop --force overriding a mismatch. You spotted that the re-check was being skipped in the exact window it exists for.

The escape hatch is narrow in the right way. I ran the matrix:

case	exit	PID file
unprobeable + --force	2	cleared
survived + --force	2	kept
mismatched + --force	2	kept
unprobeable, no --force	2	kept

survived staying put is the one I most wanted to check, since clearing it would have been the orphaning bug again under another name.

The atomic write lands through os.replace, the .tmp suffix keeps it out of the *.pid glob, and a failed write cleans up after itself. read_pid_files logs before it unlinks anything.

I checked that the tests actually bite. Reverted both source files to 5c52e88 and reran: 8 fail, all pass again on the fixed tree. Your note about test_temp_file_is_not_left_behind passing either way holds up, it is in the 10 that passed.

741 passed and 3 skipped across test_cli and test_web here. ruff check and ruff format --check clean on both changed files.

On the exit code

Keep 2. Clearing a record is not the same as confirming a stop, and returning 0 there would put back the same "report success you cannot back up" habit this PR exists to remove.

The wedge is gone either way, which I checked: the first stop --all --force exits 2 and clears the entry, the next exits 0 and reports nothing running. CI recovers without the command lying about what it did.

Two doc gaps I would like before merge

docs/cli-reference.md still says "The stop command reads these PID files, sends SIGTERM to the process, and cleans up the file." That is the behaviour you just replaced. The options table lists only --port and --all, and the exit codes are not documented anywhere. The CHANGELOG entry is good, but people tend to read the reference page.

The --force help says nothing about it now being able to delete a PID file for a process that may still be alive. Your own warning text says "it is now untracked and must be stopped by hand", which seems worth a line in --help.

Carried over, not blocking

DEBUG-level logging swallowing real HTTP failures, the run_id fallback that cannot fire, the De Morgan at rung 3, the terminate_process docstring, and dead remove_pid_file. All fine as follow-ups.

One of them got slightly more relevant: outcome is still a bare string tested with in (...) at four sites, and "mismatched" makes that a four-value set now. Still not blocking.

Sort the two doc items and I will approve.

[Frank Li (franklixuefei)]

Both doc items are done in 1446a1d, and thank you for checking the matrix rather than the summary -- the survived row was the one I most wanted a second pair of eyes on too.

docs/cli-reference.md

You were right that it documented the behaviour this PR replaced. It still said stop "reads these PID files, sends SIGTERM to the process, and cleans up the file", which is the single-signal, report-success-without-checking path #344 is about. The page described the bug.

Rewritten:

• How It Works now covers the ladder, each rung named with what it actually sends (POST /api/kill, then SIGTERM/CTRL_BREAK_EVENT, then SIGKILL/TerminateProcess), and the rule that a PID file is removed only once its process is confirmed gone.
• Options gains --force and --json, which were both missing.
• Exit Codes is a new table. I added a line on why 2 is not a synonym for "failed to signal" -- it means we could not prove the process is gone, which is why a survivor and an unprobeable run both land there.
• Identity and --force is a new section. The confirmed/unconfirmed/mismatched distinction is what decides whether --force does anything, and it had no prose anywhere outside the source comments.

I verified each mechanism claim against the source rather than the docstrings before writing it -- rung 1 is POST /api/kill at app.py:1435, rung 2 branches on sys.platform at :1457, rung 3 lands in terminate_process which is SIGKILL on POSIX and OpenProcess+TerminateProcess on Windows.

--force help

Now says the part your own warning text already said:

Force-terminate even when the run's identity cannot be confirmed. Dangerous:
the recorded PID may have been recycled onto another process. Does not
override a confirmed mismatch, which blocks every rung. Also clears the PID
file of a run whose liveness cannot be probed at all -- if that process is
still alive it becomes untracked and must be stopped by hand.

On exit 2

Agreed, keeping it. "Clearing a record is not the same as confirming a stop" is the better statement of what I was reaching for.

Carried over

Understood on all of them, and I have added the outcome string set to my list -- you are right that "mismatched" making it four values moves it from tidy-up toward worth-doing. Happy to take it as a follow-up rather than growing this PR further.

804 passed across tests/test_cli and tests/test_web, ruff and format clean. The branch is rebased onto current main.

[Jason Robert (jrob5756)]

Pushed 8b8f2d6 to your branch to clear the conflict, since it needed a judgement call rather than a mechanical resolution. All nine checks are green and the PR is MERGEABLE.

The complication was #406, which landed after your last merge and inverted the console default to markup=False. Styling now goes through styled("<template>", value), which parses the conductor-authored template but inserts values verbatim. The stop ladder predates that, so its output was f-strings with inline markup, and taking your side of the conflict unchanged would have regressed #406 inside the code #344 added.

It is not cosmetic. A workflow name reaches those messages through Path(entry["workflow"]).stem, so deploy[prod] is read as a style tag and dropped by a parsing console, or printed with raw [green] scaffolding by a non-parsing one. Given that the whole job of these messages after #344 is naming which run survived so it can be stopped by hand, a mangled name defeats the feature. Every con.print/console.print in the stop path now uses styled() where a value is interpolated and Text.from_markup where the string is static.

Worth saying plainly: I first assumed a naive resolution would slip through CI, and that was wrong. Your test_no_markup_string_literal_reaches_a_sink guard from #406 catches it and points straight at app.py:1587. The guard did its job; I just had to do the conversion.

Other resolutions:

• bg_runner.py took main's version wholesale. You threaded run_id into write_pid_file; main does the same and adds stderr_log/stdout_log, so main's supersedes yours.
• pid.py auto-merged, keeping main's new log-path fields next to your atomic write and announced deletes.
• test_stop_ladder.py: write_pid_file no longer takes log_file, so the round-trip test asserts the fields main replaced it with. That also retires the dead-field point from the first round.
• docs/cli-reference.md kept your rewritten section and folded in main's sentence about the PID file recording run_id and the capture-log paths. Main's "sends SIGTERM ... and cleans up the file" sentence is gone, since that is the behaviour you replaced.
• CHANGELOG.md kept both entries.

I also added TestWorkflowNamesArePrintedLiterally, which drives a bracketed name through the already-exited, survived and unconfirmed rungs plus the mismatch refusal and asserts the name survives with no markup leaking. It fails on the unconverted resolution, so the next rewrite of this file cannot quietly undo it.

Please give the merge commit a look, particularly the bg_runner.py call since I dropped your version of it for main's. If you are happy with it, this is good to go from my side.

[Jason Robert (jrob5756)]

LGTM. Approved!