fix(pipeline-conductor): put the process age on the banned probe line

[chenmingwei23]

Problem / Motivation

The fleet probe prints BANNED pid=<pid> rule=<regex> cwd=<class> when it finds
a banned command, and prints the same line again every cycle that process is
still alive. A pid number alone cannot tell two very different things apart: the
process the conductor already stopped is still running, or that pid was freed and
a brand-new process took the recycled number. Measured tonight: a pid the probe
reported banned on three cycles in a row was still alive holding 404 worker
processes, load at 71, memory down to 15G -- and the line read the same as a
first sighting.

Why it matters

The conductor reads this line to decide whether to act. When "already handled,
ignore" and "still burning the host" produce the same line, the reader picks one,
and the wrong pick either re-stops a healthy owner or leaves a runaway running.
Issue #8192 calls this out as item 2: the banned signal collapses no-answer and
answer into one output.

A conductor patrolling with a five-day-stale copy of this probe read a banned line for a process that was a shell wrapper and had already exited. One line misled four ways at once: the pid was meaningless because the process was gone; cwd=unknown said nothing; the match was wrong at the root because that stale copy predates the shell-wrapper fix (#8736); and a sibling reading printed cwd=fleet, which names a worktree a reader uses to decide WHICH worker to stop -- but a shell and a real test run in that worktree share the same cwd, so it accused an innocent session. The age= field this PR adds resolves the gone-or-alive axis on the current probe: age=?s says the process is gone, a growing age=<secs>s says it is still running. It does not touch the cwd attribution -- that is the matcher's job and #8736 fixed it upstream; the age is the orthogonal fact the reader still lacked. (Measured on the conductor's own five-day-stale install.)

What changed (motivation -> approach -> change)

Symptom: a re-emitted BANNED line is unreadable across cycles. Root cause: the
line carries the detection but nothing about the process's own history, so a
recycled pid and a persisting pid look identical.

Approach: add the one fact that separates them -- the process's age. An age that
grows between cycles is one unkilled process; a small age under a re-appearing
pid is a fresh violation. Age is a fact about the running process, so it needs no
new state file and no second writer, which keeps the probe read-only outside
--mark-handled (a documented invariant).

Change: a _proc_age_secs helper reads /proc/<pid>/stat field 22 (starttime)
and /proc/uptime -- both world-readable, like the cmdline this scan already
trusts, so it survives the same access asymmetry that makes cwd/exe fail for
another user's process. The emit becomes
BANNED pid=<pid> rule=<regex> cwd=<class> age=<secs>s, or age=?s when the age
is unreadable (the process already exited -- the expected reading for a
short-lived runner). It is one appended field, not a new report format. SKILL.md
gains the field in the output block and a sentence in the BANNED action row on
how to read age= across cycles.

Deliberately out of scope: issue items 3-7. Item 1 (shell-wrapper mismatch)
already landed in #8736. The other item this dispatch flagged -- an unharvested
terminal report ageing into IDLE after it scrolls past tail_bytes -- is a
different mechanism (terminal-report stickiness) and is left for a separate change.

One more read makes the whole record trustworthy. The scan reads several /proc files for one pid at different instants, so a pid recycled partway through would splice one process's identity onto another's fields -- the fidelity fix's own fidelity defect. The probe captures the process starttime (a boot-relative incarnation token) BEFORE any other per-pid read and re-reads it before emitting. On a mismatch, unreadable token, or None token the WHOLE record is withheld: cwd drops to unknown (the non-stopping class, so a spliced record can never stop an innocent worker -- the field a stop is reserved for) and age to ?s. pid and rule still print, so the violation is not dropped. No new output shape: age=?s and cwd=unknown already existed.

Every read in the emission path is bracketed by that one token -- enumerated so a reviewer can see there is no third unguarded read:

read	file	bracket	on mismatch
start_tok	/proc/<pid>/stat	captured first	(the anchor)
rule + argv	/proc/<pid>/cmdline	inside start..end	record withheld
program base	/proc/<pid>/exe	inside start..end	record withheld
cwd class	/proc/<pid>/cwd	inside start..end	cwd=unknown
end_tok	/proc/<pid>/stat	start==end proves one incarnation	cwd=unknown, age=?s
age	/proc/<pid>/stat + /proc/uptime	re-bound to start_tok inside _proc_age_secs	age=?s

starttime is monotonic per boot, so a recycle anywhere in the window changes it and is caught; a recycle back to the same starttime is impossible. The residual is therefore only the microseconds between capturing start_tok and the first read under it, and even that is bounded: the /proc walk runs every probe cycle and age is designed to be read as a SEQUENCE across cycles, so the worst case is one misleading line, once, in a field whose whole purpose is cross-cycle comparison -- it self-corrects on the next pass. A reader deciding whether to act on a single line should read age as a sequence, not a snapshot.

On the #8343 question -- why not bind a start token and refuse, as #8467 (Refs #8343) does for the signed pid mapping. The answer is that this PR does both, on different axes. #8467's consumer makes a kill decision on one identity, so a stale mapping must be refused. This probe's consumer is a reader who must still act and needs to know WHICH case they are in, so the age is reported, not refused. But the token is what makes that age trustworthy: starttime is read to VALIDATE that the age and the cmdline describe one process (mismatch -> age=?s), exactly #8467's technique, used to make the age meaningful rather than to replace it. The token is not an alternative to the age; it is its precondition.

Tests

test/test_pipeline_conductor_probe_banned_age.py, six cases over a fake
/proc (the KIROCREW_PROBE_PROC_ROOT seam the script already exposes):

• a fleet-owned unbounded pytest alive ~600s prints age=600s on its line;
• a pid whose stat is gone (exited mid-scan) prints age=?s and still emits
  the line -- age never blocks the signal or crashes the scan;
• _proc_age_secs resumes the field parse after the LAST ), so a comm holding
  spaces and parentheses does not shift the starttime field;
• with os.sysconf unavailable (Windows), the helper returns None and the caller
  emits age=?s -- the field is an explicit unknown, never a guessed number and
  never absent.
• with the incarnation token mismatched (pid recycled between the cmdline and age
  reads), the helper returns None so the caller emits age=?s rather than a
  spliced age, and the matching token still yields the age;
• end to end, a scan whose second starttime read differs from the first prints
  the BANNED line with age=?s.

Commands run in the worktree, base origin/main e7db5f5b8:

timeout 900 python3 -m pytest -n0 test/test_pipeline_conductor_probe_banned_age.py -x -q
timeout 900 python3 -m pytest -n0 test/test_pipeline_conductor_probe_roundtrip.py test/test_pipeline_conductor_skill_contract.py -q
black --target-version py310 --check <both changed .py files>
isort --check-only <both>
flake8 <both>
mypy src/kiro_crew/builtin_skills/pipeline-conductor/scripts/fleet_probe.py

Results: 6 passed (new file), 93 passed (existing probe + skill-contract), black
/ isort / flake8 / mypy clean. Mutation-verified: reverting the emit to the old
bare line reds test_banned_line_reports_the_process_age while the existing
tests stay green; and reverting the no-sysconf guard to a guessed tick rate reds test_age_is_none_without_a_clock_tick_rate; and disabling the incarnation-token check reds test_age_refused_when_the_incarnation_token_moved and the recycled-mid-scan end-to-end test.

Manual verification

N/A -- unit coverage over the fake-/proc seam exercises the emit and the age
helper directly, including the unreadable-age and paren-heavy-comm edge cases.

Related Issues

Refs #8192

Pattern harvest

Pattern: a signal built to resolve a pid-identity ambiguity was itself assembled from two unsynchronised /proc reads of that same ambiguous pid, so the fidelity fix carried a fidelity defect -- a recycled pid between the cmdline read and the age read splices one process's identity onto another's age. The general shape: when a fix reads a mutable identifier more than once to describe one entity, bind the reads to an immutable incarnation token (here process starttime) and refuse to combine them across a mismatch.

Rule candidate: review-prompt -- for any change that reads /proc/<pid> (or any reused OS handle) more than once to build one record, ask whether the reads are pinned to a single incarnation, since the pid can be recycled between them.

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 45a34e7a433c7f976486184673643af19486d3dd — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

A stateless, fail-safe field that resolves a documented cross-cycle ambiguity; the incarnation token is the right precondition, not scope creep.

The one behavior change beyond the field itself — token mismatch demoting cwd to unknown — is in the fail-safe direction (a spliced record can only lose a stop, never cause one against the wrong worker), is self-correcting on the next cycle, and is documented in both SKILL.md and the diff. No code parser consumes the BANNED line, so appending a field breaks nothing, and the field is never omitted, so a reader can't misread absence as "new process." The "Manual verification: N/A" is acceptable here: the only CI-unreachable branch (no os.sysconf) is pinned by deleting the attribute, which runs on every platform.

[DESIGN-REVIEWED] 45a34e7

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 45a34e7a433c7f976486184673643af19486d3dd — this comment is updated in place on each push.

Review details

No findings.

The sole candidate — the negative-age clamp at fleet_probe.py:1184 reporting age=0 instead of age=?s — cannot ground (a) a concrete input that occurs in practice. Both uptime and starttime are boot-relative and monotonic, read from the same /proc, and _proc_age_secs only reaches the clamp after re-verifying starttime_ticks == expected_start (a live, matched incarnation). For age = uptime - starttime_ticks/hz to go negative, the process would have to have started after the current uptime — impossible for a running process whose incarnation was just confirmed against the same monotonic clock. The discovery pass itself scored this "low" and conceded it could not construct a non-corrupt scenario; the negative branch is unreachable defensive code, and the field is advisory metadata in a read-only probe regardless. Dropped under Step 1.

[OPUS-REVIEWED] 45a34e7

Verdict parsed from the review's SHA-scoped output markers for commit 45a34e7a433c7f976486184673643af19486d3dd.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 45a34e7a433c7f976486184673643af19486d3dd: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 45a34e7a433c7f976486184673643af19486d3dd and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 45a34e7

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 45a34e7a433c7f976486184673643af19486d3dd: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 45a34e7a433c7f976486184673643af19486d3dd — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks are done. The change holds up: the age field has checkable provenance (linked issue #8192, a measured incident, tests that fail on base), the SKILL.md update is the mandated same-commit doc sync, the standalone stdlib-only script can't reuse the package's canonical age helper, and the bracket enumeration in the code matches the description. The one rider — the incarnation-token check that downgrades cwd to unknown on a mid-scan pid recycle — is declared, derived from an OS rule, and adds no new output shape.

First-Principles-Verdict: PASS

Confirm issue #8192 item 2 names this banned-line ambiguity — the linked provenance is the only fix premise not checkable in-repo.

Not justified as shipped

2. rides along — a second fix (withholding a spliced record's cwd) beyond the titled age fix; declared, harm named (a spliced cwd=fleet triggers the stop reserved for that class), no new output shape.

What this change ships

Intent: let the conductor tell an unkilled banned process from a fresh offender on a recycled pid — a FIX.

1. BANNED line carries age=<secs>s, or explicit age=?s when unreadable (never omitted) — justified
2. A pid recycled or exited mid-scan now prints cwd=unknown instead of a spliced class — rides along
3. SKILL.md teaches reading age= across cycles and widens the cwd=unknown row — justified
4. Two script-internal /proc helpers (starttime token, age) — justified

The helpers are the sixth independent starttime field-22 parser (grep starttime: 5 existing files — session_pid.py, platform_compat.py, acp/liveness.py, acp/client.py, crash_dump_store.py), but the probe is a standalone stdlib-only script, so "use the canonical one" is not reachable; not a finding.

[FIRST-PRINCIPLES-REVIEWED] 45a34e7

[chenmingwei23]

Verification record

Verified independently at head c30dc887c6a8235303f0844b526c8b54b77add82, resolved
once and used for every check below. 55 checks green, one red, and the red is not
this pull request's.

The red, attributed. Backend Lint & Type Check (3.12) fails on
test/test_pipeline_conductor_agent.py: history narration counted 5 against a
baseline entry of 4. The gate's own annotation says it plainly -- "grew from 4 to 5
on the base branch; this diff adds none of the matched lines."

Traced to a single commit by counting at each of that file's last five: everything up
to and including e2efb9dc8 counts 4, and aa75a403c -- fix(pipeline-conductor):
stop the fleet probe naming a shell as the tool it wraps (#8736), 00:44:51Z -- counts
5. The line it added is The wrapper exemption requires that kernel answer and no longer accepts. That same commit also drifted fleet_probe.py, which was filed
as #9372 and fixed by #9393 at 07:04Z; this file stayed invisible for eight hours
because nothing touched it. The condition is systemic rather than local -- #9384's
census finds 33 files off their entries -- and it is tracked in #9384, #9385 and
#9387.

The other eight conjuncts. No lane pending. All five review lanes success, with
GPT 5.6 reporting no blocking findings and its [GPT-REVIEWED] marker naming this
exact head rather than a predecessor. No unresolved review threads. Refs #8192 and
Refs #8343 rather than Closes, which is correct: #8192 carries seven items and
this addresses one, with the shell-wrapper item already landed in #8736. Tests were
run one file at a time with -n0 spelled explicitly.

What the change earned along the way

The review chain on this pull request produced two findings in the emission path, and
the way it closed matters more than the diff.

GPT's first finding was that cmdline and the age were read at different instants, so
a recycled pid could splice one process's identity onto another's age. Its second was
that the fix had moved the window rather than closed it -- the token was captured
after the cmdline read.

The response was not the one-line reorder either finding asked for. The token is
now captured before every per-pid read, re-checked before emitting, and passed as a
required parameter so the signature enforces the binding rather than a line order.
On mismatch the derived fields are withheld, not merely the age. The comment at the
emission site enumerates all three reads -- cmdline, cwd, exe -- and states that
the token brackets the whole record. Had the literal fix been taken, cwd and exe
would still be unbracketed and a third finding was available to whoever looked next.

The unknown rendering also collapses three distinct causes -- no /proc on the
platform, an unreadable token, a mismatched incarnation -- into one age=?s. That is
right: the reader's question is whether the age can be trusted, not which of three
reasons made it untrustworthy.

Not merged by this pipeline. Reporting a verified state; the merge decision is a
human's.

[chenmingwei23]

Verification record -- re-certified after rebase

Verified independently at head 45a34e7a433c7f976486184673643af19486d3dd, resolved once
and used for every measurement below. This supersedes the record posted against
c30dc887c: a rebase creates a new head, so every lane re-ran and nothing carried over,
including the AI review verdicts.

The lane that blocked this pull request is green. Backend Lint & Type Check (3.12)
= success. It had been failing on drift this diff did not cause -- test/test_pipeline_conductor_agent.py
had grown from 4 matched narration lines to 5 on the base branch, and the gate blocks
every diff touching the file regardless of whether that diff added the line. PR #9456
reworded the inherited line on main (no longer accepts to rejects, present tense,
explanation preserved), and rebasing onto it cleared the red.

Board. 56 checks green, zero failing, zero cancelled, zero pending. GPT 5.6 Review
reports no blocking findings with its [GPT-REVIEWED] marker naming this exact head --
checked specifically, because that comment is rewritten in place on each push and a stale
verdict leaves no visual trace. Opus 4.8 Review, Design Review,
First Principles Review and UX Review all green. Coverage Gate,
Testpaths Coverage Gate and Coverage Combine green.

The diff is unchanged from what was certified before the rebase, which I checked
rather than assumed: 4 files, +393/-6, one commit -- the skill file, fleet_probe.py,
and the two test files. base_sha equals main's head, so this is zero commits behind.

Comment-history count back at baseline. violations_in_source on
test/test_pipeline_conductor_agent.py reports 4, measured with the repository's own
scripts/check_comment_history.py rather than a grep -- the checker scopes to comments
and docstrings only, so a hand-rolled scan over all lines disagrees with it. Worth noting
that this diff adds 13 lines to that file and the count is still 4, which is what proves
none of the added lines match a pattern. Had any matched, the gate would be accusing this
diff rather than the base.

Tests. timeout 900 python3 -m pytest -n0 test/test_pipeline_conductor_probe_banned_age.py -x -q
-- 6 passed. Single file, no xdist.

Linkage. Refs #8192 and Refs #8343, deliberately not Closes: #8192 carries seven
separate items and this addresses one of them.

For whoever merges

This pipeline does not merge. The pull request needs an approving review; nothing is
pending on the author's side.

[NicholasRBowers]

Tier 1 auto-approve: fix (4 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix with a clear root cause — a bare BANNED pid= probe line cannot distinguish a still-running offender from a fresh violation on a recycled pid; adds an incarnation-token-bracketed age= field, read-only, fail-closed to age=?s.