Skip to content

mtcollins1 SOL collector: open silent stdin (not /dev/zero); watch yields ObservationChannelLost on collector loss - #12434

Merged
gunbai-bot[bot] merged 35 commits into
mainfrom
session/swift-wolf-904
Sep 29, 2026
Merged

gunbai-bot[bot] merged 35 commits into
mainfrom
session/swift-wolf-904

Conversation

@gunbai-bot

@gunbai-bot gunbai-bot Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Review 5333949616 (head 640c93c)

The watcher is one owned sibling, supervised (P1).

  • The step starts the watcher with a fresh token and records the child as <pid> <starttime>. The boot starts only once the readiness file holds this step's token and that child is alive; otherwise the step refuses before any BMC contact.
  • Before each further BMC effect (the attach and the handoff), the boot requires the record to name the same live process (mtcollins1_sol_observer_refusal). A gone, replaced, zombie or unobservable watcher refuses the effect. It stops effects; it does not power the machine off.
  • The trap releases the collector, asks the watcher to stop, waits mtcollins1_sol_notice_final_allowance, and collects its exit status. The boot's and the watcher's statuses are kept apart. A failed, killed or unfinished watcher is annotated with both, and fails the step even when the boot command succeeded.
  • Timing: mtcollins1_sol_notice_ready_allowance (900 s) and mtcollins1_sol_notice_final_allowance (10 s) are terms of mtcollins1_boot_step_timeout_minutes. The step's ready loop and trap wait, and the drain's pass count, are all derived from them.
  • Executed against the regenerated step script (only the two gunbc invocations replaced with inert stand-ins, under bash -eo pipefail):
Controlled history Boot ran Step exit Annotation
ready, stop, success yes 0 none
exits right after starting no 1 not recorded / result 42 (boot 1)
never ready no 1 same
exits while the boot runs yes 1 result 42 (boot exited 0)
stop, then reports failure yes 1 result 42 (boot exited 0)

Sources are observed, not defaulted (P2). Each source is read as bytes, established absent (not_found), or unreadable with path and cause. An unreadable client, incident, request-history or pid source is published as an error notice and keeps the watcher's result from being success. An unreadable request history makes each exit's ordering unresolved, never "no request was made".

Completion is judged on the delivering pass (P2). A pass returns the notices it observed, its deliveries and its unreadable sources, and the result is computed from that one snapshot. A collector exit is owed while the pid file records a collector. After the stop request the drain continues until a pass both observed and delivered it; an owed exit never observed is a failure. Every complete exit record is a notice, with identity collector-exited-<n>, so same-second and unknown-instant exits stay distinct. An earlier exit whose delivery failed stays demanded after a later one appears.

New controls, all passing locally with a binary built from this tree:

  • wet, 21/21: unreadable incident, request history and client file, each paired with its genuinely-absent positive and asserting the rendered cause and the final result; an owed exit never observed; two exits before one pass; an earlier failed summary retried after a later exit; the watcher-only exit drain.
  • pure, 33/33: every exit record distinct (same second, unknown instant); unreadable history is unresolved; the observer check (live / gone / reused / zombie / unreadable / empty record).

Also fixed: extdeps.bmc.ipmi ipmitool_sol_activate_line now pins its filtered list's type. A checker rule that came in with main refuses a Present arm over an unresolved element type; this was the floor red at 1fb2981/74f9bad0.

Still narrower than a crash audit: interruption during a summary or ledger replacement is not exercised. The interrupted-delivery control removes a marker after a completed write.

Reviews 5332355313, 5333127050, 72058 and 71987 (head a140587)

Notices are a .dag process now, not shell (review 71987). gunbc.machine_intake_mtcollins1_sol_notice runs as a second gunbc run beside the boot. It publishes the hold's timestamped exit record and the frozen incident. The annotation is encoded by a new extdeps.github.log_annotations log_annotation_message_line, which follows the runner's escapeData/escapeProperty. The step shell only starts the watcher, clears its files, waits for its readiness file, and in the trap asks it to stop.

  • Detection bound (5332355313 Add SVG viz, test helpers, and makegen scaffold #1): the watcher polls once a second, independent of the boot. SMpro passes, the attach and the handoff no longer delay a report. Resolving the entry took 340 s on a loaded host, so the boot now waits (up to 15 min) for the watcher's readiness file. If the watcher never becomes ready, the step refuses before any BMC contact.
  • Delivery (5332355313 Codex/graph viz test helpers #2, 5333127050 P2): markers are kept per notice identity and per channel. A marker is written only after that channel's write succeeded. Every delivered/FAILED outcome goes to .delivery. A summary that exists but can't be read is refused, never overwritten. An exit notice's identity carries the record's instant, so a second exit in the same step is a new notice.
  • Ordering (5333127050 P2): the exit is ordered against this run's requests by their at= instants. The result is one of: after a request (requested, not proven to be the cause), before one (so not its cause), or unresolved (same second, or an instant that doesn't order).
  • Unreadable /proc (5332355313 . #3): SolCollectorUnobservable { cause }, never an exit. Only an established exit permits the final-bytes reasoning.
  • Establishment (5333127050 P1): an unrecorded collector within the allowance is publication pending. The hold's "pid not published" line or an exit record ends the wait with its cause. The banner writes an instance-bound activation receipt (<pid> <starttime> at=). The adopted route (SolAcquireBindHeldCollector) binds the capture only when that receipt names the collector it observes.
  • Scaffold triggers (72058): the sol_hold trigger names the whole lifecycle: start, identity publication, supervision and release by identity. The boot-step trigger names the watcher's start/ready/stop coordination and the all-exit-arms trap. A partial realization satisfies neither.

Executed locally: the 16 wet claims in sol_hold_stdin_wet_witness_test pass. They cover failed annotation, failed summary, unreadable summary, interrupted delivery (redelivered, not dropped), partial exit record, second exit, watcher-only exit detection with no boot watch, delayed publication, exit before banner, and adopted receipt (none / foreign / own). The 31 pure claims in mtcollins1_boot_run_witness_test pass, including new encoding and ordering claims. One real gunbc run ... mtcollins1_sol_notice_wet delivered on both channels. Also fixed: a pure claim that pinned the checkout-ref prefix broke when main added namecheap_observe (red on main as well).

Not done: interruption during a write is covered only as far as marker-after-write gives it (a duplicate, not a drop). The trap waits 8 s for the watcher. A cancellation that kills the step sooner can lose a notice that was not yet delivered.

Brief: node adhoc-a88a13f2-151 (tracked in #12423). Responds to the HOLD review on #12434 at 1da267e and to #12423 comments 5858496324 / 5858563884. Based on #12438, the narrow stdin correction split out for independent review; this PR merges that branch and supersedes its launch with the supervised one.

Admitted notification route (operator ruling, 2026-09-27)

Not ntfy, and no new credential. The prompt SOL-loss report is a typed, timestamped ObservationChannelLost incident in the boot's own observation record: the frozen <capture>.loss file, uploaded with the capture, plus the step's failure reason. It also goes out as one Actions ::error:: annotation in the live run log and a step-summary entry. Both are published before any enrichment read, teardown or diagnostic bundle.

P1-1: host bytes never decide loss; no remote cause asserted

  • ActivateHeld now separates sources. The collector's stdout (host console) goes to the capture; its stderr goes to <capture>.client, together with the hold's own gunbc-sol-hold: records (exit status, pid-not-published).
  • Loss is decided by the process alone: the owned collector is gone. The client's words only classify a loss once one is observed. A live collector is live whatever any file says.
  • The wording is neutral. "SOL session closed by BMC" is reported as what the client printed, and the text adds that ipmitool prints it "after a failed receive or send as well as a BMC close, so it does not say which side ended the session". ipmitool 1.8.19 sets bBmcClosedSession on a null recv_sol and on send/input errors.
  • Controls:
    • pure a_gone_collector_under_a_pending_capture_is_channel_lost_not_pending: a live collector plus the text stays Live;
    • wet an_end_written_before_exit_is_judged_on_the_final_bytes: the host prints the exact text on stdout and the real watch does not report loss;
    • wet an_authentic_client_exit_is_classified_by_its_own_words: the client prints it on stderr and exits 1, and the incident carries status 1 plus the neutral phrase, while the capture holds none of it.

P1-2: final bytes before a terminal verdict

mtcollins1_sol_watch_observe reads liveness first, then the capture, then the client file. A "gone" answer means the collector exited before the capture read, so that read holds every byte it will ever write, and the verdict is over the final capture. A "live" answer can only continue. That rules out the exact interleaving you named (pending snapshot, then END and exit, then gone) by construction. A decided capture still wins over a later loss.

  • Controls, both through the real mtcollins1_boot_await_capture_terminal:
    • END-before-exit: live and pending for more than one tick, then END and exit, and the watch returns the closed envelope's verdict (REFUSED, no sections). No loss, no incident.
    • incomplete-at-exit: the envelope is open and the collector leaves, so the watch returns ObservationChannelLost and the incident is frozen.
  • Limit: there is no hook to pause between the two file reads inside one tick. The in-tick interleaving is excluded by the read order, not demonstrated by a barrier.

P1-3: publish first, bound it, supervise before power

  • Freeze first. mtcollins1_boot_channel_lost writes the incident before the mc info / chassis-power enrichment. A tick decides before it takes the census SMpro pass (reordered: only a continuing tick takes it).
  • Notice. The step prelude's 1-second watcher prints the frozen incident as ::error title=mtcollins1 SOL observation lost::… once (a .noted marker) and appends it to $GITHUB_STEP_SUMMARY. The trap stops the watcher and flushes a notice not yet printed. % is escaped.
  • Detection bound. A loss is frozen within one poll cadence (15 s) of the last live observation, plus that tick's file reads, and noticed within about 1 s after that. The incident records both last observed held at and observed gone at, an interval rather than an invented loss instant. Declared window: the post-handoff SMpro pass (at_power_on) runs before the first watch tick.
  • Supervision from acquisition. mtcollins1_sol_supervised re-reads the owned collector before the media attach and again before the handoff. Loss at either point freezes an incident at that stage (SolLostBeforeAttach / SolLostBeforeHandoff) and stops the actuation, so no handoff is issued.
  • Enrichment is neutral. BmcReachability now reads BmcAnswered | BmcReadNotSucceeded { cause }. A failed mc info carries its typed cause and no claim that the BMC gave no answer.

The earlier-requested closures

  • PID reuse: the pid file holds <pid> <starttime> (/proc/<pid>/stat field 22), written to a temp name and published with mv -T. The observation and the release both require the start time to match and the process not to be a zombie. A reused pid, even another ipmitool, reads stale, never alive (a_reused_pid_is_stale_never_held).
  • Establishment cause: SolEstablishmentFailure = SolSpawnRefused | SolPidNotPublished | SolActivateRefused{IpmitoolSolActivateRefusal} | SolSessionRefused{BmcReadCause} | SolCollectorLeftUnexplained{lease, last_line}, read from the client file. The activation refusals are modeled in extdeps.bmc.ipmi from ipmitool 1.8.19's own messages.
  • Pid-publication orphan: the supervisor is the child's unreaped parent, so it kills exactly that child and records it (wet a_launch_that_cannot_publish_its_pid_stops_the_child_it_created).
  • Cleanup on cancel/timeout: teardown runs ReleaseHeld after the BMC deactivate. The step's EXIT trap runs the same data row (mtcollins1_sol_hold_release_script, no ad-hoc kill $(cat pidfile)). The executed cancel control caught a real defect in my first trap. After the watcher had already exited post-notice, kill $SOL_LOSS_WATCH failed under set -e and aborted the trap before the release. Every fallible trap command now carries || true.
  • Declared residue: the runner's final SIGKILL escapes any trap.

Side-chat finding (#12423 comment 5859407540): our own teardown, dated

  • Timestamped exit record. The supervisor writes gunbc-sol-hold: collector exited status=<n> at=<UTC>. The incident renders "exited with status N at T".
  • Capture kept before any teardown. mtcollins1_sol_teardown_record copies the capture and the client diagnostics to <file>.pre-teardown before the deactivate. They are uploaded with the capture (same artifact glob).
  • Our own deactivate is a distinct, attributed event. Before issuing it, the teardown appends gunbc-workflow: SOL deactivate issued by this run's teardown at=<UTC> to <capture>.workflow. Only the workflow writes that file, so it never races the supervisor's appends. A client close line and exit after that instant are the consequence of our act, not evidence the BMC dropped the session. In runs 36333687404 and 36335369059 the workflow deactivated at the watch timeout before re-reading the collector, so their untimestamped close line cannot be dated before that act. "SOL died about 12 s into Linux" is unproven, and this PR does not assert it.
  • A quiet SOL is expected, not a loss. The census cmdline makes tty0 the /dev/console, so casper/initramfs output goes to KVM. Only the owned process exiting decides loss. A live, silent collector stays pending (a_live_collector_with_no_output_is_pending).
  • Control: wet our_own_deactivate_is_recorded_before_it_is_issued_and_the_evidence_is_kept. The snapshot holds the pre-teardown bytes and not the later close line, and the workflow file holds the timestamped act.

Evidence (all run locally: claim_batch built from main e66a6a6; the required CI check runs no claims)

suite result
test.claim.machine_intake.mtcollins1_boot_run_witness (pure) 26/26 PASS
test.claim.machine_intake.sol_hold_stdin_wet_witness (--wet, real launch + real watch against probe collectors) 6/6 PASS
mutation: host capture text may trigger loss an_end_written_before_exit… FAIL
mutation: incident freeze removed a_collector_gone_with_the_envelope_open… FAIL
mutation (#12438): one NUL written to fd 3 stdin claim FAIL

Executed step controls: the emitted fleet-converge.yml prelude lines, run verbatim in a harness with a supervised fake collector.

case notice collector after exit
A: loss frozen mid-step, then TERM once, 559 ms after the freeze; summary written dead; pid file removed
B: no loss, INT none dead
C: loss frozen 0.5 s before normal exit flushed by the trap dead

All six wet identities are enrolled in the floor's local-repo wet lane: a route-gap expectation, WetScheduledClaim rows, and the ci_layer_roots LocalRepoWetLane row. The hermetic route cannot run them (first effect DirWithTemplate, no mock). CI's floor therefore executes them for real; before this they refused as a route gap (floor run 36345313509 on #12438).

.github/workflows/fleet-converge.yml is regenerated through tools.generated_artifact_gate main_wet_one.

Deliberate scope and named open gaps

  • Reconnect: none, by design. A lost channel is terminal for the attempt. No capture segment is appended or spliced, and a refused attempt is never revived.
  • Notification beyond the run (push) is not built. The admitted route is the run's own log annotation and summary (ruling above). A push route would belong to the operator-alert lane and needs its own grant.
  • Typed identity carrier for the deployed ipmitool: open. The srv1 identity (1.8.19, 1.8.19-7ubuntu0.24.04.3 arm64, sha256 prefix 84b8c7b03972e809, operator-reported) is recorded as rationale on gunbc.machine_intake_sol_hold. Its trigger is a consumer: a preflight that refuses when the runner's ipmitool identity differs from the build this analysis covers.
  • Accepted-then-loss is covered only at the pure fold. The real-watch control drives the REFUSED arm (a closed envelope with no sections), because an accepting capture needs a full qualification fixture.

Not merging.

🤖 Generated with Claude Code

…ch yields ObservationChannelLost on collector loss

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head verdict: HOLD / REQUEST_CHANGES

Reviewed 1da267e, base e66a6a6; the head was unchanged at publication. All five changed files were inspected, together with acquisition, lifecycle observation, actuation, conclusion and SMpro consumers. This is a source verdict, not permission to boot/reset/deactivate a live machine.

Keep the silent FIFO correction. I independently exercised the fetched launch script on local Linux x86_64, replacing only its ipmitool executable token with an explicit absolute path to an inert C probe (not a PATH shim; no network, BMC or credential access). With the new script: select on fd0 timed out after 3 s, zero bytes, no EOF, fd0 O_RDWR on an unlinked FIFO, fd3 closed, and no FIFO pathname left behind. The /dev/zero control supplied 32 NUL bytes in a bounded read; /dev/null returned EOF. This supports the narrow stdin repair. It does not execute the .dag dispatcher/watch or the deployed ARM64 ipmitool.

Source blockers

1. P1 — console data is being promoted into a transport event and a BMC-cause claim. sol_hold.dag still merges stdout/stderr into one capture; sol_channel_reading searches that entire capture for the substring SOL session closed. A healthy target printing that text is therefore classified as lost and its pending attempt is aborted. Even an authentic ipmitool diagnostic does not establish that the BMC initiated closure: upstream IPMITOOL_1_8_19 lib/ipmi_sol.c sets bBmcClosedSession after a null recv_sol result and after certain send/input-processing errors, then emits this diagnostic. sol_channel_loss_text nevertheless says the BMC closed the SOL session. Preserve source-tagged collector diagnostics/exit separately from host bytes; classify the observation without asserting an unproved remote cause. Add a healthy-host-output control containing the exact diagnostic text and an authentic client-exit control. Source reference: https://raw.githubusercontent.com/ipmitool/ipmitool/IPMITOOL_1_8_19/lib/ipmi_sol.c (ipmi_sol_red_pill, around lines 1500-1577). This is upstream-tag grounding, not a completed Ubuntu patch audit.

2. P1 — final-capture/exit race can turn completed qualification into ObservationChannelLost. await reads the capture first, then reads collector liveness, and judges the original capture. The admitted interleaving is: capture snapshot is pending; the child appends the final valid envelope/END and exits; liveness observes it gone; the old pending snapshot is immediately refused, with no final drain/re-read. The pure WatchAccepted arm does not protect this interleaving because it never receives the final capture. Finalize the owned child/output streams and adjudicate the final attempt-owned bytes before making pending-plus-exit terminal. Do not erase an actual gap or revive an already terminal attempt. Add deterministic barrier-controlled cases for both END-before-exit and genuinely incomplete capture at exit, through the real watch.

3. P1 — observed loss is still not promptly published, and detection has blind windows. After reading channel state, await can execute the census SMpro pass before watch_tick. Its loss branch then waits for mc info and chassis status to construct a String/ProcessExit. Actuation subsequently performs parameter/power reads and SOL teardown; conclusion performs further diagnostics before writing the bundle/receipt. There is no new immediate notification dispatch, delivery outcome, incident timestamp, or separately enforced detection bound on this path. Increasing the step timeout is not a loss-detection bound. The first watch also follows the initial post-power SMpro pass, and there is no new loss guard during attach/handoff after acquisition. Publish/freeze the typed incident through the admitted operator-reporting route before optional diagnostics, bound notification independently, and supervise the owned collector from acquisition through the watch so loss before handoff prevents further handoff. A slow/unreachable BMC must not postpone the initial SOL-loss notification. Preserve independent management/power readings as enrichments; they cannot replace the incident or qualification evidence. These requirements are already in #12423 comments 5858496324 and 5858563884, not a new general-platform prerequisite.

Controls: the evidence is narrower than the claim names

The committed stdin claim at mtcollins1_boot_run_witness_test.dag checks materialized argv strings, not stdin execution. I independently ran a mutation inserting printf '\\000' >&3 between the successful fd3 open and unlink: it retained every positive string check and neither forbidden path, so the claim's Boolean conditions remain true, while the inert child actually read one NUL byte. This is an executed transport mutation plus a direct evaluation of those string conditions, not a claim that I reran claim_batch. Keep the useful old-argv RED, but commit an executing zero-byte/non-EOF control tied to the actual realization.

The new lifecycle controls call pure folds with authored LeaseRunning* values. They do not exercise /proc observation, a dying child, the watch loop, slow diagnostics or notification. The PR body correctly discloses that limitation. Also, a_decided_capture_is_not_unmade_by_a_later_channel_loss currently checks only a constructed TerminalRefused, not TerminalAccepted or the final-read race. Add the production-route controls that make removing the supervisor/notification/final-drain connection fail, rather than treating 22/22 as that evidence.

Adjacent-class answer: STILL OPEN, with a second executed counterexample

The class is broader than non-idle stdin: an observer without established, instance-bound lifecycle evidence is treated as a trustworthy live channel, and an observation failure is turned into a device claim or delayed final result.

  • Existing mtcollins1_boot_sol_held_from_observations still accepts any readable cmdline containing ipmitool. There is no process start-instance/owned-child check, operation/target match, activation-established state, or retained exit/signal. Reused PID/another ipmitool invocation can appear held, and a live client still establishing a session is not established observation. These are pre-existing weaknesses now consumed by the new critical decision, not newly discovered changes to unrelated code. Supervise the owned instance; process existence and transport establishment are different facts. Quiet established sessions must remain admissible.
  • I also executed the new launch with a valid capture path but PID destination deliberately made a directory. FIFO creation/open/unlink succeeds, the child starts, echo $! > "$5" fails, and the launcher exits 1 while the child remains alive (it was still present and completed the probe 3 s later). mtcollins1_sol_activate_held returns immediately on !spawned.success; the outer failed-acquisition arm does not roll back that child. This ownership/publication-failure class was already possible with the old launcher and is NOT being called a newly introduced regression. It remains relevant to the required owned-resource repair: fail the publication or setup after spawn and terminate/reap only the child actually created; retain the initiating error and cleanup disposition. Do not solve it with global kill/deactivate.
  • After fixing the provenance issue, do not keep the unconditional BmcUnanswered wording for every failed mc-info invocation: command/session/auth/local-tool failure does not necessarily prove that no BMC response occurred. The same file already distinguishes definitive no-answer causes. Retain the underlying typed cause and neutral wording where the answer is unknown.

I am not making the general service-realization capability, host-binding refactor, or entire 100-host matrix a prerequisite for an independently reviewed stdin-only patch. A narrowed transport correction can be reviewed on its own head; this combined head is held because its new watch decisions have concrete defects. Full collector/supervisor/notification closure still requires the integrated controls above.

Evidence and scope

The supplied srv1 identity is recorded as operator-reported: ipmitool 1.8.19; dpkg 1.8.19-7ubuntu0.24.04.3 arm64; binary SHA256 prefix 84b8c7b03972e809. Update the PR's stale 'version unknown' statement; do not ask the operator to repeat this observation. I read upstream 1.8.19 source and verified the Ubuntu package version exists, but did not independently fetch the running binary or complete its distribution-patch comparison (source-package download was unavailable here). This identity is not proof that /dev/zero caused the original incident or that a BMC reset is indicated.

22/22 .dag claims remain lane-reported; I did not rerun the full compiler/claim suite. My executions were the five isolated launch variants described above; all controlled children were reaped. At the last check, compiler and clippy had succeeded, floor and emit-build were still running on this SHA. The HOLD is source-based, not waiting for a CI color.

Source: HOLD. Adjacent class: OPEN. Live-attempt readiness: HOLD. No live hardware operation, workflow dispatch, merge, reset or SOL deactivation was performed by this review.

fn sol_channel_reading(observed: HeldLeaseObservedState, capture: String) -> SolChannelReading {
match observed {
LeaseRunningExpected =>
if string_contains(s: capture, pattern: sol_session_closed_line) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not classify host console text as a collector transport event

This searches the whole capture, but ActivateHeld still redirects >> "$4" 2>&1. Target stdout containing SOL session closed therefore aborts an otherwise live pending attempt. Even the genuine ipmitool diagnostic does not prove BMC-initiated closure (the 1.8.19 loop emits it after null recv_sol / certain other errors). Consume source-tagged collector lifecycle/diagnostic events separately from host bytes, preserve the raw diagnostic, and use neutral cause attribution. A live target printing this exact text must not report channel loss.

smpro_census: SmproReading,
) -> TerminalWatch {
let capture = Filesystem.Read(path: capture_path)
let channel = sol_channel_reading(observed: mtcollins1_sol_observed_from_pid_path(pid_path: pid_path), capture: if capture.success { capture.content } else { "" })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Drain/re-read the final capture after observing the owned collector exit

The capture snapshot is taken before this liveness read. The child can append the final valid END/envelope and exit between the two reads: the code then combines old TerminalPending with newly lost liveness and returns ObservationChannelLost without reading the completed file. The pure terminal-priority test cannot cover this ordering. Finalize/drain the owned output and adjudicate final attempt-owned bytes before making pending-plus-exit terminal; add a deterministic barrier-controlled integration case for this exact interleaving.

match watch_tick(verdict: verdict, channel: channel) {
WatchAccepted => TerminalWatch { outcome: ExitSuccess, census_ended: ended, smpro_census: census_pass }
WatchRefused { causes: causes } => TerminalWatch { outcome: exit_failure(reason: mtcollins1_boot_terminal_refusal_text(causes: causes)), census_ended: ended, smpro_census: census_pass }
WatchChannelLost { loss: l, last: why } =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Publish/freeze loss before diagnostic effects, with an independent notification bound

This cannot return the incident until mtcollins1_boot_channel_lost finishes mc-info and chassis reads. The preceding census_pass can also execute the entire SMpro pass before this branch; callers then perform more reads/teardown/conclusion before output. No prompt notification dispatch/delivery outcome is introduced. A dead observer plus slow BMC still delays the very event the operator needs immediately. Emit and retain the typed loss first through the admitted reporting route; let bounded management/power enrichment follow without blocking it. Test the real watch with a child dying while diagnostics stall.

!string_contains(s: script, pattern: "/dev/zero")
&& !string_contains(s: script, pattern: "/dev/null")
&& string_contains(s: script, pattern: "mkfifo -m 600")
&& string_contains(s: script, pattern: "exec 3<>")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Execute stdin behavior instead of proving an argv spelling

I ran the fetched launch against an explicit inert C process: the FIFO correctly supplied zero bytes/no EOF for 3 seconds, /dev/zero supplied NULs, and /dev/null EOFed. But inserting printf '\\000' >&3 after the open preserves every string condition here while the process reads one NUL. Retain the old-argv RED, but add a committed executing behavior control tied to the actual realization. The separate local receipt is useful evidence; this claim alone does not enforce its named property.

…alive-loss exit line

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copy link
Copy Markdown
Contributor

Reproduction spelling correction for review 5331626127 and its stdin inline comment: the tested Bash command contains one backslash before 000:

printf '\000' >&3

Insert that after the successful exec 3<>"$in" and before the unlink. My review text overescaped this shell literal. The actual executed probe used the spelling above: one NUL was read, while every current argv-string predicate remained true. The evidence and exact-head HOLD verdict are unchanged.

…stop the child when its pid cannot be published; executed stdin control

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copy link
Copy Markdown
Contributor

Research addendum for the operator's new KVM evidence is now on #12423, comment 5859407540. Important correction to the historical premise: in BOTH inspected runs (36333687404 and 36335369059), the actual workflow explicitly invokes SolDeactivate after the eight-minute terminal watch, and collector state is not read again until afterwards. The last kernel [12.2...] timestamp does not date an untimestamped client-close diagnostic below it. Early unsolicited exit remains possible, but must be established with pre-teardown capture/exit evidence; timeout cleanup could instead have produced the final close line. Please do not keep 'BMC closed twelve seconds into Linux' as an independently proved causal receipt.

The full note includes the exact UTC operations, upstream ipmitool issue #17 / 1.8.19 receive-path evidence, separate console-routing hypothesis, and controls for quiet live session -> workflow-owned cleanup versus genuine early exit. Instrument the actual owned client cause/time and source-tagged diagnostics, not only the text 'closed by BMC'. The screenshot supplies a live-root refusal, not evidence of a BMC restart. Existing source HOLD 5331626127 is unchanged; no hardware action was performed.

Brian Searls and others added 6 commits September 27, 2026 20:19
…nostics, liveness-before-bytes, incident frozen and annotated before diagnostics

Merges the narrow stdin correction (#12438) and builds on it: pid+starttime
identity, establishment cause, supervision before attach/handoff, loss decided
by the process not by text, neutral client-report wording, the ObservationChannelLost
incident frozen to the attempt and noticed via the step's ::error:: annotation
(operator ruling: no ntfy, no new credential), owned release on teardown and
on the step's EXIT trap.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ap expectation, schedule rows, exclusion row)

Floor run 36345313509 refused both identities: their first effect is
shell.Mktemp.DirWithTemplate with no mock_response, so the hermetic route
never reached a verdict. The wet lane runs them for real.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… deactivate recorded as an attributed workflow event

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-head rebind — HOLD / REQUEST_CHANGES

Reviewed 5d39a7d, currently open/non-draft/mergeable. #12438's narrow stdin correction is independently APPROVED at 87214b1 (review 5332331851); this verdict is on the combined supervision/notification route and does not retract that approval.

Accepted corrections

The actual launch separates stdout from client stderr/exit records, and host text no longer decides loss while the owned process is observed live. Wording no longer treats the client's session-closed string as proof of a BMC-initiated failure. Reading lifecycle before capture repairs the originally reported final-capture/exit interleaving when the lifecycle observation actually establishes exit. PID+starttime is an improvement over name/PID alone. The live watch decides before enrichment on its loss branch, freezes an incident before management reads, and the step has an actual annotation/summary consumer. The admitted operator route is the run log/summary, not ntfy; I am not requesting new alert credentials. Retain the timestamped workflow-deactivate event and pre-teardown snapshots.

1. P1 — The declared detection bound is false during in-watch diagnostics, not just the disclosed initial pass

In WatchContinue, a live observation with CensusRunning and SmproNotTaken enters mtcollins1_boot_smpro_pass, then sleeps the full poll cadence, THEN reads lifecycle again. If the collector exits immediately after seen, detection is delayed by that entire pass plus the cadence and file reads. The side watcher only watches .loss; the supervisor writes its exit to .client but does not freeze .loss, so neither notices during this blocking interval. Attach and handoff similarly have only point-in-time guards, not continued detection during their potentially lengthy operations.

The source/PR claims one poll cadence from the last live observation plus the tick's file reads. That excludes an actually reachable, potentially long diagnostic block. Reordering diagnosis after the decision protects a loss ALREADY observed; it does not protect one that occurs during diagnosis. The initial post-power SMpro window is disclosed, but this additional in-watch window remains.

Keep lifecycle supervision independent of optional/blocking BMC diagnostics (or provide an actual bounded interleaving that enforces the selected detection deadline throughout the claimed interval). Test a controlled collector death after a live tick while the census diagnostic is deliberately stalled; the first incident/annotation must meet the declared bound without waiting for that pass. Also account for the acquisition-to-first-watch and attach/handoff intervals in the stated scope. No new hardware attempt is needed.

2. P1 — 'Noted' is published before notice delivery; errors or interruption can suppress the only live report

The emitted sol_loss_notice creates $SOL_LOSS_NOTED BEFORE reading the incident or writing the annotation and summary. A failed output, failed read, or trap killing the watcher between that marker and printf makes subsequent calls refuse because the marker exists. The function is also invoked in an && context, so set -e does not turn each intermediate failure into a correctly retained delivery result. A successful final summary write can mask a failed annotation.

I independently executed the exact notice function locally against an inert incident, redirecting annotation stdout to /dev/full. It printed a write-error, returned 0, left the noted marker, and a second invocation returned 1 without retrying the annotation; the summary happened to succeed. This was an isolated shell control, not the .dag engine or GitHub run log. All files were in review scratch; no workflow or host was touched.

mtcollins1_sol_loss_freeze also writes the visible file directly; a concurrent watcher can observe it nonempty before the full write completes, mark it noted, and publish a partial incident. If the freeze fails, mtcollins1_boot_channel_lost still enters enrichment and no prompt alternative signal is provided.

Publish the complete frozen incident atomically, distinguish its publication from each delivery outcome, and do not mark a failed/interrupted delivery completed. Make teardown and the watcher coordinate so a kill-between-marker-and-output cannot drop the report. Retain delivery failures distinctly without hiding the original loss; retry/deduplicate by incident identity where required. Add partial-publication, failed annotation, failed summary, and interruption-at-delivery controls through the emitted path.

3. P2 — Unreadable lifecycle evidence is still promoted to confirmed collector exit

mtcollins1_boot_sol_held_from_observations returns false for stat/cmdline read failures. mtcollins1_sol_observed_from_collector maps a nonempty PID file plus false to LeaseRunningStale; sol_channel_reading maps every non-running state to SolChannelGone. A live child whose /proc evidence could not be read is therefore reported as 'gone' / 'no longer running'. The final-bytes argument also relies on this stronger interpretation, although failure to read process state does not establish that the producer has stopped writing.

Preserve known exit/zombie/instance replacement separately from unreadable/indeterminate process observation and its cause. Both may withhold safe qualification, but only established exit permits the final-output reasoning. Add live-but-unreadable stat/cmdline and definite-exit positive/negative pairs at the real observation fold. The publication path must report the precise observation failure rather than manufacture an exit.

Adjacent class / remaining scope

Process identity and session establishment remain distinct. Acquisition still sleeps, tests (pid,starttime,cmdline contains ipmitool), and returns success; a live client still establishing a connection is not evidence that SOL activation succeeded. This earlier-requested establishment obligation is not closed by adding better failure-word parsing on the non-live arm. Retain it explicitly and test delayed/failed activation before claiming the observer ready for handoff.

The pre-teardown snapshots are a useful discriminator, not proof that every event after a timestamp was CAUSED by the following deactivate. There is still a window between recording intent and issuing the call; keep the attribution at observed/requested strength. Do not reinstate the historical unsupported 'died at 12 seconds' conclusion.

Release currently checks starttime then signals by numeric PID; full atomic process-handle ownership and interruption-safe cleanup remain relevant to the lifecycle lane. Do not call generic final-SIGKILL cleanup complete. I am not making a general platform/harness implementation a prerequisite to correcting the concrete publication and observation defects above.

I inspected exact-head launch, lifecycle, acquisition, watch/freeze, emitted-prelude model and call sequencing. I did not independently run the 26/26 or 6/6 .dag suites, regenerate YAML, dispatch, merge, contact BMC or deactivate SOL. The reported wet controls are meaningful but do not cover the stalled-diagnostic and failed-delivery counterexamples.

Source: HOLD. Adjacent class: OPEN — observer uncertainty and failed notification can still become definite or silently completed events. Live-attempt readiness: HOLD.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review: HOLD remains at unchanged 5d39a7d

The current API head is still this SHA. It is not a newly repaired/rebased successor of my REQUEST_CHANGES 5332355313. I re-read the executing watch, lifecycle observation and generated notification prelude. The three findings remain; the stock-image audit and narrow #12438 stdin fix do not change them.

  1. Detection bound still excludes reachable diagnostic work. mtcollins1_boot_await_capture_terminal takes a live observation, can execute the census SMpro pass, then sleeps before the next lifecycle read. The separate shell watcher observes only the .loss file; it does not detect collector exit while the main watch is blocked in those reads. The initial post-power pass is another declared blind interval. Deciding before optional work after one sample does not supervise the next interval. Establish the real detection bound and an executing loss-during-slow-diagnostics control, rather than asserting one poll cadence.

  2. Incident publication/delivery is still lossy. mtcollins1_sol_loss_freeze writes directly to the public watched path. sol_loss_notice creates .noted BEFORE annotation/summary delivery. I independently reran the exact function in the same conditional-call shape, with the annotation destination /dev/full and a writable summary. It returned success on the first call (the last summary write succeeded), left .noted, and refused a second attempt; the annotation was never delivered. This used an inert incident and temporary files, no real secrets or GitHub command delivery. Publish a complete incident atomically and record each actual delivery outcome after it is established; do not suppress retry on failed/partial delivery. Preserve the underlying incident if reporting fails.

  3. Unknown process observation still becomes confirmed exit. The /proc read-success flags become the collector_held Bool and the non-held route; sol_channel_reading maps every state other than LeaseRunningExpected to SolChannelGone. A failed process-state read therefore becomes a gone-collector claim, and the liveness-first/final-capture argument then assumes a final output boundary that was not established. Distinguish inaccessible/unread/inconsistent ownership observations from an observed exit of the owned instance; keep the actual failure cause. A safe withholding on uncertainty need not claim the process died.

Keep the accepted source separation, neutral client-cause wording, process start-instance identity, pre-teardown evidence and narrow FIFO correction. #12438 remains independently source-approved. The adjacent class is still trusting an observer without complete lifecycle evidence, then conflating detection, durable incident publication and successful operator delivery.

New forensic evidence does change the historic incident description, NOT this source verdict: I downloaded the actual 36335369059 receipts ZIP (artifact 10938081294), verified its recorded SHA256, and found the reported kernel command line and Error sending SOL data: FAIL/closure text, but no Casper live-root refusal in its three files. The separate KVM transcription is not yet bound to this attempt. The client failure lines are untimestamped, so they still do not establish an exit at host uptime 12.2 s or before the workflow's intentional teardown. A separate #12423 correction records that provenance.

Source: HOLD; REQUEST_CHANGES 5332355313 remains the binding decision. Adjacent class: OPEN. Integrated live attempt: not cleared by this review. I did not rerun claim_batch, dispatch, merge or perform any BMC/host operation. This COMMENT re-review intentionally does not manufacture a new source delta or duplicate the existing request-changes review.

Brian Searls and others added 2 commits September 28, 2026 01:06
…elivery markers set only after a successful write; unreadable /proc is its own arm; establishment by the operational banner

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HOLD — exact-head rebind at e21b740

Reconfirmed identical to refs/pull/12434/head before publication. This is not a repeat of the unchanged-head verdict: the new source substantially repairs the previous three mechanisms, and I independently exercised its revised notification/launch statements. Three concrete issues remain below.

Accepted repairs

  • The one-second step watcher reads the supervisor's exit record independently of the .dag watch and SMpro/media operations. A raw collector-exit warning can therefore precede the later classified incident. Reporting the raw observation first, without inventing a boot verdict, is appropriate.
  • Per-notice/per-channel markers now follow the output write, and the trap kills and waits for the watcher. I executed BOTH /dev/full controls against the exact generated sol_deliver function: annotation-failed/summary-good and annotation-good/summary-failed. Each returns nonzero, creates only the successful channel's marker, and succeeds on a subsequent healthy retry without re-sending the already-marked channel. This closes the original simple counterexample; it was not merely inspected.
  • ProcReadRefused preserves error_kind/error. Permission/I/O failures now produce SolCollectorUnobservable, not a fictional exit; the rendering distinguishes unobservable from final-after-exit. The incident stages then renames its file. Keep these changes.
  • For a newly launched collector, the operational banner is a sensible establishment observation for the versioned client contract, and line-buffering addresses its file-buffering delay. The literal upstream 1.8.19 banner occurs after activation checks and before its receive loop. No physical client was run in this review.

1. P1 — asynchronous PID publication is refused immediately, and adoption still bypasses establishment

ActivateHeld returns once its supervisor is backgrounded; PID/start-time publication happens later inside that supervisor. mtcollins1_sol_activate_held immediately calls mtcollins1_sol_await_establishment. That wait retries only SolCollectorHeld-without-banner. SolCollectorUnrecorded goes directly through other to SolNotEstablished even with the full allowance remaining.

I executed the fetched launch unchanged except for its declared executable input, which named an absolute-path inert C probe. In 5/5 executions, launch returned 0 while the PID file was still absent; shortly afterwards it was published, the operational banner appeared, and the child exited normally. This is an executed launch/publication ordering observation, combined with source reasoning about the .dag classifier; I did not run the .dag engine or claim the full wet witness failed locally. Interpreter overhead may hide the interleaving, but cannot make it impossible.

Model launch-pending/publication as part of this attempt's bounded establishment transaction, or make the launch acknowledge publication before returning success. Only a missing record attributable to that in-progress owned launch may stay pending; do not reinterpret arbitrary disappearance after establishment as healthy waiting. Include a deterministic delayed-publication positive, publication failure, deadline, and exit-before-establishment.

The same invariant is still missing on the OTHER success route: SolAcquireBindHeldCollector truncates the evidence and returns ExitSuccess without checking any activation-established fact; mtcollins1_sol_supervised checks process liveness. A still-negotiating owned process can therefore bypass the new banner requirement when adopted. Establish or consume an instance-bound establishment receipt before admitting that route. Do not simply wait for the original banner after truncating it; an already-established client does not print it again. Test both acquisition routes, not only await_establishment on a fresh probe.

2. P2 — summary-body failure is still marked delivered

The summary channel uses:

if { printf HEADER; cat "$1"; printf '\n'; } >> "$GITHUB_STEP_SUMMARY"; then ...mark delivered...

Only the final printf determines that group's status. The conditional/OR calling context also means set -e is not a substitute for explicit status propagation.

I exercised a real filesystem interleaving, with no command/PATH shim: the summary destination was a FIFO so its open could be paused after annotation completed; I removed the source notice before allowing the summary group to open. cat reported ENOENT, but the trailing newline succeeded. sol_deliver returned 0, created the summary marker, and recorded summary delivered although the summary contained only its header and blank lines. The annotation had correctly received the original inert body. A missing/unreadable source is a relevant delivery failure, not complete delivery.

Require successful acquisition of the exact notice bytes and successful header/body/trailer writes before committing that channel's marker. Preserve/retry failures and keep identity tied to the same immutable snapshot, rather than separately hashing one file state and reading another. Add source-read and partial-output failures alongside the now-passing /dev/full controls. This is the same producer-status class, not a demand for a new notification service.

Primary shell reference: https://www.gnu.org/s/bash/manual/bash.html (compound lists and the -e exceptions for tested functions).

3. P2 — the independent exit notice manufactures the event order it was intended to recover

sol_exit_event samples the latest workflow-act line WHEN THE NOTICE IS BUILT and unconditionally says the collector exit was 'after this run's last recorded request'. It never compares or otherwise establishes the order of the exit and request. A valid interleaving is exit at t0, local teardown request at t1, watcher samples both at t2.

I supplied those exact inert records (exit 11:00:00Z, teardown 11:00:01Z) to the fetched function; it printed exit after the later request. '(requested, not proven to be the cause)' limits causality but does not repair the incorrect chronological claim. Retain the two records independently and either establish ordering or report what had been recorded by notice time without asserting prior/later. Equal-resolution or incomparable timestamps must stay unresolved. Test both orderings and the coincident/unknown case; do not let the collector-exit finding be reattributed to cleanup merely because cleanup happened before the watcher noticed it.

Adjacent-class residue: observer identity and evidence lifetime

I also exercised a stale-input specimen: an old .client exit and .workflow record present when the prelude starts are accepted as this attempt's notice; exit.event then remains fixed and the later genuine exit is never independently reported. The prelude clears capture/loss/delivery but not client/workflow until the .dag acquisition runs. Qualification: this job does run the pinned actions/checkout with its default clean=true, which normally removes old untracked files. The specimen therefore demonstrates a prelude/recovery precondition, not a claim that every ordinary clean dispatch inherits stale files. Keep that assumption explicit or bind fresh source paths before starting the watcher; account for adopted/surviving collectors and records recreated after checkout. Do not erase a previous attempt's evidence merely to make the new subscription appear clean.

The broader class is start requested vs observer established, event occurrence vs time noticed, and successful final output vs complete evidence delivered. No additional BMC simulator, hardware experiment, image change or whole-repository cleanup is required for these repairs.

Execution evidence and scope

Independent local work: Bash 5.2.37 on Linux x86_64; exact fetched sol_deliver/sol_exit_event functions against inert temporary files, both /dev/full positives, controlled source-read failure and event-order/stale-input specimens; five launches of the fetched ActivateHeld script with an explicit inert executable. All probe collectors terminated and their supervisors recorded exit; no BMC, network, credentials, real ipmitool execution or .dag dispatcher was used. Source and results are retained in the chat evidence archive.

The new wet establishment claim is actually listed in local_repo_wet_schedule; that establishes scheduling, not a completed verdict. I accept the author's EAGAIN/unrun disclosure and do not turn the old six-claim result into a pass for the new claim. At the checked SHA compiler and generated were successful; floor and emit-build were still running. I did not fetch a completed named wet-claim result or regenerate the YAML. The unrelated Spark-floor issue cannot discharge the source counterexamples.

Source: HOLD at this SHA. Adjacent class: OPEN as named. Integrated live-boot HOLD remains. The earlier narrow #12438 approval is not retracted. No merge, enqueue, workflow dispatch, power action, media write, BMC reset or live SOL operation was performed.

mtcollins1_sol_await_establishment(pid_path: pid_path, capture_path: capture_path, remaining: remaining - 1)
}
}
other => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] A just-started supervisor may not have published the PID yet

ActivateHeld returns before its background supervisor publishes pid/starttime. This arm immediately refuses SolCollectorUnrecorded even with allowance remaining. In 5/5 isolated executions of the actual launch against an explicit inert executable, return=0 preceded PID-file creation; the banner then appeared normally. Represent this attempt's spawn/publication-pending state or synchronously acknowledge publication, with a bound and a deterministic delayed-publication control. Do not turn arbitrary missing ownership into indefinite pending.

Comment thread .github/workflows/fleet-converge.yml Outdated
export GUNBC_MTCOLLINS1_SOL_PID_FILE="$SOL_PID_FILE"
SOL_LOSS="$SOL_OUT.loss"; SOL_CLIENT="$SOL_OUT.client"; SOL_WORKFLOW="$SOL_OUT.workflow"; SOL_DELIVERY="$SOL_OUT.delivery"
: > "$SOL_LOSS"; : > "$SOL_DELIVERY"; SOL_NOTICE_DIR="$RUNNER_TEMP/mtcollins1-sol-notice"; rm -rf "$SOL_NOTICE_DIR"; mkdir -p "$SOL_NOTICE_DIR"
sol_deliver() { [ -s "$1" ] || return 1; id=$(sha256sum < "$1" | cut -c1-16) || return 2; if [ ! -e "$SOL_NOTICE_DIR/$id.annotation" ]; then m=$(head -c 4000 "$1" | tr -d '\r\n'); m=${m//\%/%25}; if printf '::%s title=%s::%s\n' "$2" "$3" "$m"; then : > "$SOL_NOTICE_DIR/$id.annotation"; echo "$id $2 annotation delivered at=$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$SOL_DELIVERY" || true; else echo "$id $2 annotation FAILED at=$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$SOL_DELIVERY" || true; fi; fi; if [ ! -e "$SOL_NOTICE_DIR/$id.summary" ]; then if { printf '### %s\n\n' "$3"; cat "$1"; printf '\n'; } >> "$GITHUB_STEP_SUMMARY"; then : > "$SOL_NOTICE_DIR/$id.summary"; echo "$id $2 summary delivered at=$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$SOL_DELIVERY" || true; else echo "$id $2 summary FAILED at=$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$SOL_DELIVERY" || true; fi; fi; [ -e "$SOL_NOTICE_DIR/$id.annotation" ] && [ -e "$SOL_NOTICE_DIR/$id.summary" ]; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] A successful final newline can hide a failed summary-body read

The summary if { printf; cat "$1"; printf '\n'; } tests only the final command's status. I synchronized on the completed annotation, held summary open using a FIFO, removed the notice source, then released the FIFO: cat failed ENOENT, but this returned 0, marked summary delivered, and wrote only a header/blank lines. Require the complete notice acquisition and every write to succeed before its channel marker. Both ordinary /dev/full retry controls now pass; this is the remaining same-class partial-delivery failure.

Comment thread .github/workflows/fleet-converge.yml Outdated
SOL_LOSS="$SOL_OUT.loss"; SOL_CLIENT="$SOL_OUT.client"; SOL_WORKFLOW="$SOL_OUT.workflow"; SOL_DELIVERY="$SOL_OUT.delivery"
: > "$SOL_LOSS"; : > "$SOL_DELIVERY"; SOL_NOTICE_DIR="$RUNNER_TEMP/mtcollins1-sol-notice"; rm -rf "$SOL_NOTICE_DIR"; mkdir -p "$SOL_NOTICE_DIR"
sol_deliver() { [ -s "$1" ] || return 1; id=$(sha256sum < "$1" | cut -c1-16) || return 2; if [ ! -e "$SOL_NOTICE_DIR/$id.annotation" ]; then m=$(head -c 4000 "$1" | tr -d '\r\n'); m=${m//\%/%25}; if printf '::%s title=%s::%s\n' "$2" "$3" "$m"; then : > "$SOL_NOTICE_DIR/$id.annotation"; echo "$id $2 annotation delivered at=$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$SOL_DELIVERY" || true; else echo "$id $2 annotation FAILED at=$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$SOL_DELIVERY" || true; fi; fi; if [ ! -e "$SOL_NOTICE_DIR/$id.summary" ]; then if { printf '### %s\n\n' "$3"; cat "$1"; printf '\n'; } >> "$GITHUB_STEP_SUMMARY"; then : > "$SOL_NOTICE_DIR/$id.summary"; echo "$id $2 summary delivered at=$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$SOL_DELIVERY" || true; else echo "$id $2 summary FAILED at=$(date -u +%Y-%m-%dT%H:%M:%SZ)" >> "$SOL_DELIVERY" || true; fi; fi; [ -e "$SOL_NOTICE_DIR/$id.annotation" ] && [ -e "$SOL_NOTICE_DIR/$id.summary" ]; }
sol_exit_event() { [ -e "$SOL_NOTICE_DIR/exit.event" ] && return 0; l=$(grep -m1 -F "gunbc-sol-hold: collector exited status=" "$SOL_CLIENT" 2>/dev/null) || return 1; a=$(grep -F "gunbc-workflow: " "$SOL_WORKFLOW" 2>/dev/null | tail -n 1); if [ -n "$a" ]; then c="after this run's last recorded request: $a (requested, not proven to be the cause)"; else c="no teardown or release had been requested by this run; the boot watch classifies it against the capture at its next observation"; fi; printf 'mtcollins1 SOL collector exited -- %s -- %s\n' "$l" "$c" > "$SOL_NOTICE_DIR/exit.staged" && mv -f "$SOL_NOTICE_DIR/exit.staged" "$SOL_NOTICE_DIR/exit.event"; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Sampling a later cleanup request does not place the exit after it

This says 'after this run's last recorded request' whenever the workflow file currently contains any request. Exit at t0, teardown at t1, notice poll at t2 therefore gets reversed. Executed with exit 11:00:00Z and teardown 11:00:01Z: the emitted text says after. Preserve independently timed events or describe the request as present when the notice was built; do not infer ordering/cause from adjacency or polling time. Add both-order and same-resolution controls.

…d publication; instance-bound activation receipt for adopted collectors; exit ordered against requests by instant

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot pushed a commit that referenced this pull request Sep 28, 2026
…l the loss-ask wet control

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…rate fleet-converge.yml; rename a binding that shadowed the none keyword

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot pushed a commit that referenced this pull request Sep 28, 2026
…llows #12421's mode; regenerate fleet-converge.yml

kvm_still, boot_run and diagnostic_bundle witnesses pass locally (gunbc run --claim-run).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ffold triggers name the whole hold/release lifecycle and the trap's watcher coordination; avoid the shadowed ends_with; boot checkout-ref claim asserts the boot arm, not other modes

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HOLD — exact-head SOL notice/lifecycle rebind

Reviewed a140587, reconfirmed identical to refs/pull/12434/head immediately before publication. The comparison to held e21b740 includes the reported main integration; I reviewed the current topic patch and the new notice/coordination consumers, not every unrelated main-parent change.

The original delayed-PID and adopted-activation specimens are repaired, and replacing the old shell delivery/classification functions with the .dag notice fold is the right direction. However, the new process and final-drain boundaries contain concrete failure paths. HOLD for the three findings below, not because the general BMC harness or a physical trial is unfinished. No live host/BMC operation is needed to discriminate these.

1. P1 — readiness survives watcher death, and the watcher result never joins the step result

The generated prelude waits for $SOL_OUT.notice.ready, testing that file BEFORE kill -0, and after the loop checks only file existence. It never observes the watcher's status while the foreground boot runs. The EXIT trap polls kill -0, optionally sends TERM, and removes files, but never waits on the owned child or consumes its result. A .dag watcher returning ExitFailure because notices remain undelivered therefore does not refuse the step. Polling whether it is alive is not waiting for/adjudicating its outcome.

Independently executed against the exact generated coordination statements, with only the two absolute $ROOT/target/release/gunbc invocations supplied by an explicitly created inert process implementation in a private temporary workspace (no PATH shim, evaluator, network, credentials or BMC):

scenario watcher status inert boot reached/completed step status
ordinary ready -> stop -> success 0 yes / yes 0
publish ready, then exit before parent proceeds 42 yes / yes 0
exit unexpectedly while boot is running 42 yes / yes 0
receive stop, return failure 42 yes / yes 0

All three failed-watcher cases also produced no failure annotation from the coordination. This demonstrates the actual shell lifecycle/status boundary, not an execution of the .dag watcher or hardware boot.

Required repair: join readiness to this owned process's live establishment, observe unexpected sibling exit during the period it is required, and consume its final result after the stop request. Preserve the initiating boot result as well as observer/cleanup results; a required observer failing cannot be hidden by a successful foreground command or by rm being the trap's last command. Prevent new hardware effects once required observation is unavailable; this is not permission to blindly power off the host. Keep stop/kill/reap ownership explicit and preserve evidence if forced termination is needed. This is one owned sibling's lifecycle, not an infinite hierarchy of watchdogs or a new job.

Execute the three cases above through the production coordination, plus never-ready and ordinary successful stop. The readiness file alone is not an ongoing health receipt.

Related timing integration: the new startup loop admits up to 900 seconds, while the existing mtcollins1_boot_prelude_allowance is 120 seconds and the step timeout derivation has no new watcher-preparation term. The reported 340-second resolve already exceeds that prelude number. Account for the actual preparation and final-drain allowances in their existing timing authority/projection when repairing this lifecycle; do not claim the old derived bound includes them, or call a one-second sleep a hard end-to-end notification bound. No arbitrary performance rewrite or automatic timeout increase is prescribed.

2. P2 — unread notice sources become absence and fabricated negative history

read_or_empty maps EVERY Filesystem.Read failure to "". sol_notices_present uses it for the collector diagnostic file, workflow-request record and frozen incident. The new summary-read code correctly distinguishes not_found from unreadable, but the source side discards exactly that distinction.

Concrete source-derived cases:

  • An existing .loss path that is a directory/permission failure yields no incident at all, rather than observation-unavailable. With a readable exit record the pass can settle and sol_notice_finish return true without ever judging that incident source.
  • An unreadable .workflow beside a valid exit becomes requests=[]; the delivered text states "no teardown or release had been requested by this run". Failed observation does not establish that historical negative.
  • If the required sources remain unreadable, an empty delivery list is all(...) == true, and expiration of the final allowance returns that value. Unobserved is therefore reported as settled.

These are analyses of the current .dag branches, not independently executed claim_batch results.

Required repair: use the existing typed file-observation boundary (read / established absent / unreadable, retaining path and cause). Preserve normal pre-event absence as pending where the lifecycle permits it, but do not discard non-absence failures or make them terminal delivery success. Publish an observer-source failure on the healthy available channel, retry under the admitted bound, and retain an unsuccessful final observer outcome when observation remains unavailable. Request history that was not read must be rendered unknown, never 'no request'. Do not let best-effort ledger failure rewrite the meaning of source availability either.

Add tests for unreadable client, incident, and request-history sources separately, with genuine absent/empty positive controls. Require both the reported cause and the final watcher result, not merely absence of a delivery marker.

3. P2 — the final drain can report an exit delivered when it never delivered that snapshot

sol_notice_finish first calls sol_notice_pass, which takes its own source snapshot and delivers it. It then calls sol_notices_present AGAIN to compute exit_seen.

The admitted interleaving is:

  1. stop is requested; the first snapshot contains no completed exit record, and no incident;
  2. sol_notice_pass returns [] (settled);
  3. the supervisor appends its complete exit record;
  4. the second snapshot sees that exit;
  5. settled && exit_seen returns true without ever delivering the exit on either channel.

The same issue occurs when an already delivered exit A is replaced by a newly observed exit B between the two reads. This is a source-derived final-drain race. The current real-watcher control waits for an eventual exit but does not deterministically place publication between these reads.

Required repair: judge delivery coverage over the SAME admitted notice population being used as completion evidence, and re-observe/drain newly published records before returning. Retain producer completion/expected-exit knowledge where it is needed; timeout cannot turn an unknown/missing required final record into success. This can be a small structured pass result with identities and delivery outcomes, not a parallel expected transcript or a new queue service.

There is a related loss in that population: sol_exit_notice_of selects only the LAST exit. If exit A's summary fails, then exit B appears before the next pass, A disappears from the demanded set and is never retried. Two complete exits already present before one poll likewise produce only the last notice. Moreover the identity uses only a second-resolution instant: distinct exits in the same second (or both with an unknown instant) collide. The existing 'later exit' control delivers A fully before appending B five minutes later, so it misses all three cases.

Keep every still-undelivered exit in the admitted population and bind notice identity to a real record/collector instance or cursor, not timestamp uniqueness. The implementation itself describes adopted/restarted collectors and claims second-exit support; alternatively, a genuinely single-exit contract must be enforced and multiple records must refuse rather than silently disappear. Preserve the no-automatic-reconnect boot policy. Add a barrier-controlled finish race, failed-A-channel then B, multiple exits before a poll, and same-second/unknown-instant controls through the actual notice fold.

Repairs accepted from the previous review

  • The fresh establishment loop now keeps SolCollectorUnrecorded pending within the allowance and terminates on explicit publication failure or an exit. It still requires held ownership plus the operational banner; a live-but-negotiating client is not admitted as established.
  • The adopted route now requires an activation receipt naming the observed pid/starttime before clearing/binding the attempt capture. It does not wait for a one-shot banner to print again. Keep the none/foreign/own receipt controls and delayed-publication controls.
  • Notice body construction and channel writes now consume an in-memory SolNotice; the old conditional shell group whose failing cat was hidden by a final newline is gone. Each channel checks its own Filesystem.Write result before its marker; marker-write failure remains explicit and retryable. The summary's non-absence read failure is preserved. The defects above concern sources, population and parent lifecycle, not a request to resurrect sol_deliver.
  • For the current writers' valid UTC-second records, the new ordering fold distinguishes before/after/same-second and keeps causal strength at 'requested, not proven cause'. This fixes the old latest-request-equals-before-exit specimen. Preserve unknown order when history or timestamps cannot be established; instant_comparable is only a shallow shape test, not complete timestamp admission.
  • The loss-source rename and separation of host capture, client diagnostics and workflow requests remain. Unreadable /proc remains SolCollectorUnobservable, not exit. Existing final-capture ordering and terminal precedence are not weakened by the rebind.
  • The lifecycle dissolution descriptions now include start/publication/supervision/release and step start/ready/stop/trap, rather than claiming the lifecycle is dissolved by a start-only emitter. The checkout-ref witness narrows its literal expectation to its actual boot-mode subject rather than maintaining a changing list of unrelated modes. Those are reasonable bounded corrections.

Adjacent class / evidence / scope

The remaining class is an observer that can disappear without its parent noticing, or certify complete delivery from missing or mismatched evidence populations. The producer/readiness receipt, source observation, notice identity, channel completion and child exit result must be consumed together. Adding a modeled process instead of shell does not by itself establish those joins.

The current interrupted-delivery test removes a marker AFTER a completed write; it does not execute interruption during the read-modify-write of the summary or ledger. Preserve that evidence limit, and do not claim the marker control proves prior delivered content survives partial replacement writes. No full transport crash-consistency audit or physical trial was performed in this review.

The reported 16/16 wet, 31/31 pure, and real notice-process run remain author-run evidence. I did not rerun the .dag suite or compile gunbc here. My four independent executions were the inert generated-coordination cases above under GNU Bash 5.2.37 on Linux x86_64; scripts/results are retained in this conversation. All controlled children finished; no live process or fleet resource was used.

At the latest reads this SHA had zero check-runs, no PR-triggered workflow run returned, and the PR reported mergeable=false. I am not assigning those statuses a source cause or treating the earlier-head CI as evidence for this head. The topic diff is 12 files; the old-head comparison also includes the reported main merge, not merely the notice edit. Several lower PR-body paragraphs still describe the removed shell watcher, old test counts and an asserted twelve-second disconnect; update the current summary rather than leaving those as apparently current facts.

Source: HOLD at a140587… for the three named boundaries. Existing narrow #12438 approval is unchanged. Integrated live-boot HOLD remains unchanged. No enqueue, merge, dispatch, credential fetch, BMC/SOL connection, reset, image change or hardware experiment was performed or authorized. The required repairs are local process/fold integration; they do not require finishing the general BMC acceptance matrix first.

Comment thread .github/workflows/fleet-converge.yml Outdated
: > "$SOL_LOSS"; : > "$SOL_CLIENT"; : > "$SOL_WORKFLOW"; : > "$SOL_DELIVERY"; rm -f "$SOL_OUT.notice."*
ROOT=$(git rev-parse --show-toplevel 2>/dev/null || pwd)
"$ROOT/target/release/gunbc" run --source-root "$ROOT/dag" --source-root "$ROOT/src/v2" --entry dag/gunbc/machine_intake/mtcollins1_sol_notice.dag --function mtcollins1_sol_notice_wet & SOL_NOTICE_WATCH=$!
for i in $(seq 1 900); do [ -e "$SOL_OUT.notice.ready" ] && break; kill -0 "$SOL_NOTICE_WATCH" 2>/dev/null || break; sleep 1; done; if [ ! -e "$SOL_OUT.notice.ready" ]; then echo "::error::mtcollins1 SOL notice watcher is not ready; refusing the boot before any BMC contact"; exit 1; fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Consume the owned notice watcher's lifecycle, not only its ready file

This tests the ready file before checking the process, then checks only file existence. No watcher status is consumed during the foreground boot or in the EXIT trap (the trap polls kill -0 but never waits/adjudicates the child's result). I executed the generated coordination with explicit inert absolute gunbc stand-ins: ready-then-exit-42, exit-42 during boot, and exit-42 after stop ALL ran/completed the inert boot and returned step status 0. Join readiness to the owned process, observe unexpected loss while required, and propagate its final failure independently of the boot's result. Keep ordinary success and never-ready controls; no hardware test is needed.


fn read_or_empty(path: String) -> String {
let r = Filesystem.Read(path: path)
if r.success { r.content } else { "" }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Do not turn unreadable incident/exit/history sources into absent notices

This fallback is used for .client, .loss and .workflow. An unreadable .loss disappears from delivery coverage; unreadable .workflow becomes a delivered assertion that no teardown/release was requested. Empty deliveries can then settle successfully. Preserve typed read/absence/unreadable observations and their causes, allow genuine pre-event absence where appropriate, and refuse final observer completion on unresolved required reads. The summary side already distinguishes these cases; apply that discipline to the sources.

// appends its record just after the reap), or until the allowance ends.
fn sol_notice_finish(paths: SolNoticePaths, remaining: Int) -> Bool {
let deliveries = sol_notice_pass(paths: paths)
let exit_seen = sol_notices_present(paths: paths).any(n => match n.kind { SolNoticeCollectorExited => true _ => false })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Final delivery and exit completion must refer to the same snapshot

sol_notice_pass may observe no completed exit and return [] (settled). If the supervisor appends the exit before this second read, exit_seen becomes true and the function returns success without delivering that exit on either channel. Return the admitted notice identities/coverage from the pass and drain new records against producer completion rather than joining two unrelated snapshots. Add a deterministic publication barrier here. Also retain older per-channel-undelivered exits instead of selecting only the newest one.

…the phase clock keeps the SOL supervision before attach and before handoff; HandoffRun carries the collector's last live instant for the watch; regenerate fleet-converge.yml

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Brian Searls and others added 2 commits September 29, 2026 12:32
… unreadable stat), like the owned check; drop the scalar settle helper for the sleep carrier inline (review 72605 on #12492)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot pushed a commit that referenced this pull request Sep 29, 2026
…trap); keep the KVM observer release in the trap; drop this stack's interim sol_hold route-gap chunk (now enrolled by #12434); the KVM wet controls sleep on a typed Second (review 72605)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot pushed a commit that referenced this pull request Sep 29, 2026
…vable); the trap keeps the KVM release's exit code (1 survived KILL, 2 unobservable) instead of reading non-zero as one cause; the channel-loss SOL control sleeps on a typed Second

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ad is not a publication failure (bash reaps it asynchronously), so the ordinary wait records its exit and the client's refusal decides the cause; an unobservable collector is pending inside the establishment allowance

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HOLD — 18ac261

Reviewed requested 16f523a plus its current successor. The sole P1 from review 5345454202 is repaired: the failed-publication supervisor no longer falls through to an unbounded wait "$p" after it has established NOT stopped. It records the residue with pid, returns without an ordinary exit record, and the exact spliced-supervisor control covers that branch. Keep that repair.

One new integration blocker is now exposed by exact-head CI, in the same SOL establishment boundary rather than in unrelated code.

P1 — a fast, known activation refusal is nondeterministically hidden by PID-publication outcome

Exact-head run 36577209770 reached the floor and failed only test.claim.machine_intake.sol_hold_stdin_wet_witness.a_refused_activation_carries_its_cause: expected passed, observed failed. Generated, emit-build and rust-unit-tests succeeded.

The source explains why this can vary with scheduling. refused_probe writes the known ipmitool activation refusal to client stderr and exits immediately. If the supervisor manages to publish pid/starttime first, mtcollins1_sol_await_establishment observes the exited collector and sol_establishment_failure parses SolActivateRefused. If the child exits before start-time publication, the supervisor writes a pid not published line. sol_establishment_failure checks the pid-not-published lines before calling ipmitool_sol_activate_refusal, so the same client refusal becomes SolPidNotPublished instead. The wet claim therefore races exactly the lifecycle scheduling it is supposed to classify.

This is not merely a flaky assertion: a real controller can refuse activation quickly too. The workflow then loses the most specific observed remote cause and reports a local publication consequence instead. For the 100-host bar, the answer must be stable under that ordering.

Repair: model/retain both observations or establish a deterministic precedence that does not discard the known client refusal. A concrete IpmitoolSolActivateRefusal observed in the client diagnostics should survive whether pid publication completed; separately retain any publication/cleanup residue if it matters. A bare reordering is acceptable only if it does not erase a surviving unowned-child obligation.

Controls: run the same known refusal in both lifecycle orders: (a) identity published before exit, and (b) exit/refusal before identity publication. Both must retain the same activation refusal; the second may additionally retain the publication state. Keep a separate publication-failure-with-no-protocol-refusal case so the generic pid failure remains discriminated. This can be an inert probe; no BMC execution is needed.

Prior findings closed

  • Failed publication now has bounded TERM/KILL observation and a returnable unresolved-residue branch.
  • The generated trap preserves activation-receipt ownership and three-valued owned checks.
  • The banner/silent fixtures are no longer the old short autonomous-sleep race and the allowance is correctly described as poll count, not a strict wall-clock deadline.

The reported targeted local controls were not completed; exact-head CI is therefore the first execution and it is red on a meaningful sibling establishment case. Under the operator's STOP → ROOT CAUSE → MODEL → PROCEED rule, do not retry this away.

HOLD at this SHA pending deterministic multi-observation refusal classification and a green exact-head wet suite. The explicit C5/C8 scaffold ruling remains separate. No merge, enqueue, dispatch, credential read, or hardware action performed.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HOLD — current head e45e0c2

This supersedes HOLD 5345454202. The unbounded-wait P1 is repaired: after the second bounded termination observation, a still-present child writes the NOT-stopped residue and exits the supervisor immediately, before the ordinary wait/exit-record path. The exact spliced supervisor control using pid 1 is a valid discriminator for bounded return, and the normal ended-child path still reaps and records its exit.

One adjacent P1 remains at the model boundary: the cleanup residue is not instance-bound once the supervisor returns.

P1 — the unresolved child identity is written as prose and then discarded

The shell now records pid not published; collector NOT stopped (survived TERM and KILL) pid=<p>. But SolEstablishmentFailure represents that state as the nullary SolPidNotPublishedUnstopped, and sol_establishment_failure recognizes it only by substring. The pid — and the start time already read into s when available — do not enter the typed outcome.

This matters precisely because the supervisor intentionally returns while the child may still exist. At that point it is no longer enough that p was the supervisor's unreaped child while the supervisor was alive. The process may later exit and its pid may be reused; a follow-up attempt or residue processor has no typed instance identity with which to distinguish the original survivor from a later process. The normal pid record was never published, by definition of this branch.

The current wet control proves bounded return and that the prose contains pid=1; it also demonstrates the gap: the typed boot-side arm would still be only SolPidNotPublishedUnstopped.

Required repair: carry the unresolved publication residue as data, not only text. At minimum preserve the pid and the observed starttime when one was available; if starttime itself could not be observed, preserve that as a distinct unobservable-identity state/cause rather than fabricating an instance identity. Thread that carrier into the establishment failure / run evidence so later logic can refuse or process the exact residue without reparsing a diagnostic string. Do not create a normal published pid record after publication failed, and do not signal a later process merely because it reused the pid.

Required controls: through the real diagnostic-to-establishment composition, require that the NOT-stopped line with a known pid/starttime yields the same typed identity, and that an unavailable starttime yields an explicit unobservable residue. A consumer presented with the same pid but another starttime must not treat it as the original survivor. Keep the existing bounded-return control.

This is the adjacent class the operator's STOP / ROOT CAUSE / BULK UP MODELING rule is intended to catch: an unresolved cleanup outcome is only useful if the identity of the unresolved resource survives the boundary.

What is closed

  • The publication-failure supervisor no longer performs an unbounded wait after declaring NOT stopped.
  • A child observed ended follows the ordinary reap + exit-record path; an unconfirmed child does not emit a false collector-exited record.
  • The outer trap's activation-receipt ownership, three-valued owned check, bounded watcher handling, and separate credential cleanup remain intact.
  • The banner controls are split by case and the source now states correctly that the establishment allowance counts paced polls rather than a strict wall-clock number.
  • The remaining hand string is now explicitly scoped to C5 background-hold plus C8 fd-redirection; whether that scaffold is authorized remains the separate pending operator ruling, not something this review grants.

Evidence

The requested targeted controls were disclosed as not locally completed. Current exact-head workflow 36597893469 was queued when I checked, so I do not claim a wet pass at this revision. I inspected the exact current successor, not only requested 16f523a; the later commits do not close the typed-residue gap above.

HOLD at this exact SHA for the one instance-bound residue finding. No merge, enqueue, dispatch, credential use, process signal, or hardware operation performed.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE-MERGE — #12434 at e45e0c2

This clears HOLD 5345454202. I reviewed the requested 16f523a repair and the current successor e45e0c2. The bounded publication-failure lifecycle is preserved; no new blocking source finding.

The remaining P1 is closed

sol_hold_supervisor_body now branches after the TERM / 2 s observation / KILL / 1 s observation sequence. If the child is still present, it records pid not published; collector NOT stopped (survived TERM and KILL) pid=<p> and exits the supervisor immediately. That exit occurs before the ordinary wait "$p" and before the normal collector-exit record, so the unresolved branch is actually bounded and returnable.

If the child is observed gone instead, execution falls through to the ordinary wait/reap and writes the actual exit record. The implementation therefore no longer equates 'KILL requested' with observed exit, and it does not fabricate an exit record for unresolved cleanup.

The supervisor body is now exposed as mtcollins1_sol_hold_supervisor_script, and ActivateHeld splices that exact rendered value rather than a test-only copy.

The new wet control exercises that exact script with publication forced to fail and pid 1 standing for a process whose termination cannot be confirmed. It requires return within an outer bound, the pid-bearing NOT-stopped residue, and absence of the normal collector-exited record. That is the discriminator requested by the HOLD. The existing TERM-ignoring-but-KILLable control remains the complementary positive.

Scope qualification: the pid-1 fixture relies on the wet runner being unprivileged with respect to pid 1. That is appropriate for the current runner route, but if this suite is ever moved into a privileged/root namespace, preserve the 'cannot be signalled/reaped' premise explicitly rather than assuming pid 1 has the same behavior there.

Earlier lifecycle repairs remain intact

The outer trap still preserves activation-receipt ownership before collector release, the owned check remains three-valued, a running watcher with unconfirmed identity is not signalled or waited on indefinitely, and credential cleanup is independent. The banner establishment cases are now split so a loaded-run failure identifies which subject failed; the allowance wording correctly says poll count rather than strict elapsed seconds.

The reported targeted local runs were interrupted before completion, so I do not treat them as passes. Exact-head workflow 36597893469 was still in progress at my check; normal required checks remain necessary. I inspected the current source and retained controls but did not execute claim_batch or the supervisor fixture myself.

APPROVE-MERGE at this exact SHA through normal CI/merge-queue gates. This source approval does not supply the still-pending explicit operator ruling for the remaining C5/C8 scaffold, nor does it authorize a live boot. No merge, enqueue, dispatch, credential use, BMC request, or hardware operation performed.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE-MERGE — #12434 at e45e0c2

This closes HOLD 5345454202. The requested 16f523a has advanced to this successor; I reviewed the current production supervisor and retained controls. No remaining source blocker from that HOLD.

The bounded publication-failure lifecycle now actually returns

sol_hold_supervisor_body now distinguishes the child that is still present after TERM, the 2 s observation, KILL, and the 1 s observation. In that branch it writes pid not published; collector NOT stopped ... pid=<p> and immediately executes exit 0 inside the branch, before the ordinary wait "$p" and exit-record append below.

So the exact state from the prior P1 no longer falls through into an unbounded wait. Conversely, if the child is observed ended, the branch does not exit early: the supervisor reaps it with the ordinary wait and writes the real collector-exited record. This preserves the important distinction between requested KILL and observed termination.

The residue record retains the only available pid because publication never succeeded, and no fabricated 'collector exited' line is emitted on the unresolved branch. That is the bounded outcome requested by the prior review.

Control shape is appropriate

an_unconfirmed_termination_is_residue_and_the_supervisor_returns executes the exact rendered supervisor body exposed as mtcollins1_sol_hold_supervisor_script, with pid 1 standing in for a live process the test user cannot terminate or reap. The control requires return within an outer 30 s bound, the residue line including pid=1, and absence of an exit record. That is a practical discriminator for the unreachable-without-privilege SIGKILL-survivor state; it does not pretend pid 1 is a launched collector.

The existing TERM-ignoring-but-KILLable control remains the complementary positive, ensuring the normal escalation still observes and records termination.

The earlier banner-fixture concern is also addressed structurally: the establishment cases are split so CI identifies the failing case, the probes are test-owned/released rather than relying on the old autonomous timing race, and the source now describes the allowance as poll count rather than strict elapsed seconds. Keep that precision.

Evidence / scope

The prior trap ownership, three-valued owned-check, post-media handoff supervision, capture-watch supervision, and checked result/cleanup join remain intact. I did not rerun the wet or pure claims locally. The author disclosed that the new targeted controls had not completed locally; exact-head workflow 36597893469 was in progress when I checked, so normal required checks remain mandatory and I am not claiming those controls have passed CI yet.

GitHub reports the current head mergeable. The narrowed C5/C8 scaffold still has only default approval; this source approval does not supply the requested explicit operator scaffold ruling or lift the separate integrated live-boot authorization hold.

APPROVE-MERGE source at this SHA, subject to normal exact-head CI/merge-queue gates and the separate scaffold authority requirement. No merge, enqueue, dispatch, credential access, or hardware action performed.

…with a binary built from the merged tree

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@gunbai-bot
gunbai-bot Bot added this pull request to the merge queue Sep 29, 2026
gunbai-bot Bot pushed a commit that referenced this pull request Sep 29, 2026
…ate fleet-converge.yml

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Merged via the queue into main with commit e0a2bdb Sep 29, 2026
5 checks passed
@gunbai-bot
gunbai-bot Bot deleted the session/swift-wolf-904 branch September 29, 2026 23:01
gunbai-bot Bot pushed a commit that referenced this pull request Sep 29, 2026
… in #12434

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot pushed a commit that referenced this pull request Sep 30, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot pushed a commit that referenced this pull request Sep 30, 2026
…rate fleet-converge.yml over the #12434 squash

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot pushed a commit that referenced this pull request Sep 30, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot pushed a commit that referenced this pull request Sep 30, 2026
…trols

The dry realization now answers the route #12434 landed:
- processes carry a start time, visible as /proc/<pid>/stat (field 22) beside cmdline; ActivateHeld
  publishes "<pid> <starttime>", sends the grounded preamble's banner to the capture and its stderr
  to the client diagnostics, and a foreign session's refusal to the diagnostics;
- one exit transition (deactivate, session drop, ReleaseHeld) removes the /proc entry and records the
  supervisor's exit line; ReleaseHeld stops only the recorded instance;
- the notice watcher the step starts before the entry is scenario state (with_notice_watcher);
- shell.Move File is a rename in gunbc.filesystem_model; ipmitool mc info answers with the two
  fields the corpus read from this controller, its layout typed TranscribedUncited.

Flips: pinned_a_healthy_census_is_refused_at_the_sol_teardown ->
a_healthy_census_completes_and_releases_its_collector (ok; deactivate, ReleaseHeld, two retiring
deletes, all under the hold). pinned_a_sol_loss_mid_boot_is_reported_only_at_the_deadline ->
a_sol_loss_mid_boot_is_reported_before_the_deadline (typed ObservationChannelLost, incident frozen,
BMC answering, teardown within 60 s of power-on). The held-elsewhere case asserts the typed cause.

claim_batch, the merged tree: matrix, realization, model and filesystem witnesses 42/42 PASS. The
eval-step drop now covers nine members (the listing case crossed the budget on the longer route),
each row re-measured; docs/design-rung-drops.md regenerated.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot pushed a commit that referenced this pull request Sep 30, 2026
…se success arm, the media-loss attribution stays on the failure arm

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot pushed a commit that referenced this pull request Sep 30, 2026
…is rendered by the world's one proc_stat_line; #12533 now carries the duplicate-listing cost row

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot pushed a commit that referenced this pull request Sep 30, 2026
…s PR's cases on it

On the merged tree all 28 matrix and model witnesses pass, the forward-jump case asserting 3 looks
(the notice-watcher wait #12434 added moved its schedule; the refusal at the look after the jump is
what it holds). Eight of this PR's cases are over 72,300 on the longer route, two more than
before (cd_error_16_with_the_host_on_writes_nothing, a_foreign_presented_image_is_not_stopped);
their rows join #12533's nine, each measured by claim_batch here, and docs/design-rung-drops.md is
regenerated at seventeen.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot pushed a commit that referenced this pull request Sep 30, 2026
…red drop; two eval-step rows

Side-chat ruling (eager-owl-205, 2026-09-30): the six cases CI measured strictly above the 302 ms
enrolment margin and under the 500 ms line (396, 394, 378, 361, 339, 332 ms, run 36648847499) are
rostered in v2.workflow.floor_enrolment_dead_band, self-staling both ways, under the declared drop
gunbc.rung_drop.mtcollins1_boot_matrix_enrolment_dead_band_observed_only. Its population derives from
those rows, and its trigger names the one capability both matrix drops wait on, now a single row
(mtcollins1_boot_matrix_native_witness_capability) that the eval-step drop also reads. The CPU is
#12434's own polling route run faithfully; ablation found no model hotspot.

The eval-step drop gains the interrupted-attempt and wrong-share cases (74,419 and 77,423 on CI),
eleven rows. Rung-drop and enrolment witnesses 19/19 PASS; docs/design-rung-drops.md regenerated.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@briansrls
briansrls restored the session/swift-wolf-904 branch September 30, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant