Repository navigation
fleet-converge: run the reset-return dispatch admission only for host_reset_return - #12419
Conversation
The build job's admission step carried if_condition: none, so every mode's dispatch paid a full gunbc run to reach ResetDispatchNotThisMode. It now carries fleet_converge_host_reset_return_step_if, derived from the one mode roster. The route witness asserts the gate. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
APPROVE-MERGE — exact head f9db76b65a8bf5291396bdbb89b91d7745b16e3b, reconfirmed as the current open, non-draft, mergeable head targeting main. No blocking finding.
The mode gate preserves the host_reset_return admission refusal. This change removes an unnecessary process invocation for other declared workflow modes; it does not turn an applicable safety refusal into a skip.
Reviewed the complete three-file delta: the modeled admission step changes only its if_condition from none to Present { value: fleet_converge_host_reset_return_step_if }, accompanied by the explanatory annotation; the generated YAML gains the corresponding one-line if: github.event.inputs.mode == 'host_reset_return'; and the existing routing witness gains an assertion of that condition. The admission invocation, input environment, timeout and error behavior are unchanged.
Safety route checked at this head:
fleet_converge_host_reset_return_step_ifis derived byfleet_converge_mode_step_if(mode: HostResetReturn)from the existing mode vocabulary. The host-reset-return JOB uses the same condition. The new step condition does NOT inspect reset_observer, so an empty observer cannot make an otherwise applicable host_reset_return admission disappear.- The admission remains in
fleet_converge_build_job_own_steps, after release build and before pack/upload. The generated step calls the originalhost_reset_return_dispatch_admission_wetentry and passes the original dispatch mode and observer through GUNBC_HOST_RESET_DISPATCH_MODE and GUNBC_HOST_RESET_OBSERVER. There is no|| true, conditional observer default, or continue-on-error added. admit_reset_dispatchstill distinguishes NotThisMode, admitted reset dispatch and unnamed observer; it trims the observer, so both empty and whitespace-only values refuse for the canonical host_reset_return mode. The wet entry mapsResetDispatchObserverUnnamedtoexit_failurewith the existing ResetObserverUnnamed diagnostic, rather than success.- The generated
host-reset-returnjob retainsneeds: [build]and the ordinary mode-only job condition, withoutalways()or another failure override. Therefore this admission's failed step fails build and prevents the reset job from running. If an earlier build step fails so admission cannot run, the same dependency also prevents the reset job. For other declared modes, skipping this one step does not skip the build job or its release-artifact handoff.
The witness change checks the REAL build-job own-step list, requiring the expected step identity, exact roster-derived gate, target entry and function, and both dispatch-input environment bindings. A missing/incorrect gate cannot satisfy that new conjunct. Existing neighboring controls still exercise unnamed reset refusal, named reset admission, other-mode non-applicability, whitespace-only refusal, and the build dependency edge. I inspected their assertions; I did not independently execute them or the stated gate-removal mutation.
The remaining build, pack and upload steps still provide the shared release artifacts; this PR neither removes those capabilities nor changes the selected jobs' consumers. The approval does not claim overall fleet-converge overhead is now zero, or that building the entire release-bin roster is globally minimal. It accepts this narrowly removed not-this-mode invocation.
Non-blocking import hygiene: the new witness reference to HostResetReturn is not in that file's explicit gunbc.fleet_converge_workflow import list. Include it alongside the other named modes when touching the witness next; this is not a reason to widen or change the gate.
Evidence scope: source and generated-workflow review only. I did not rebuild, regenerate, run claim_batch, dispatch a workflow, or independently measure the reported approximately four-minute saving. The after-merge measurement remains the named admission step reporting skipped on a non-reset dispatch; no live post-change receipt exists in the supplied evidence.
At the latest exact-head check, clippy was successful; compiler, floor and emit-build were still in progress. Require the normal exact-head and merge-queue gates to finish green before landing.
|
STRATEGY REVIEW — APPROVED WITH AMENDMENTS. This is approval of the fleet-converge overhead work plan, not an exact-head approval of unsubmitted implementation and not authorization to execute a live boot/reset/IAM operation. Existing source approval of #12419 is unchanged. Decision: retain the three independent lanes; choose route B (the existing fleet-converge build job as the on-demand keyed release-pack producer). Do not widen the required merge-group/floor/witness job roster or add R2 publication credentials there merely to manufacture a cache producer. This is a scoped first implementation choice given the submitted producer inventory, not a claim that route B is globally optimal for every future workload.
Acceptance of the reuse implementation needs a real cold build/publish -> separate-dispatch restore -> actual consumer path, plus mutated-input, wrong-key, tampered/missing-bin, invalid-producer and eviction controls. Measure warm total cost (identity + lookup + transfer + verification + handoff), not just a skipped Cargo step. Same-key reuse does not make a first dispatch at a new build identity free. Keeping the existing complete pack is a safe initial scope; it does not establish that every member is necessary for every mode forever.
Reported baseline is 11 minutes across the three stated pre-hardware steps; savings must not double-count the pre-entry work already removed when an entire inapplicable step is skipped. Optimize time to first authorized hardware action and time to completed verified observation separately, and report cold and warm cases. No live post-change latency or cache-hit result was measured by this review. |
|
Follow-up to the approved overhead strategy (comment 5857457629), at the operator's explicit request to make the manual boot-retry residue modeled and wired: tracked in #12423. This does not revoke this PR's source approval or make all recovery work a dependency of the mode-gate change. Please route #12423 to sleek-fox-141's attach change and the boot-waits/recovery child, link their actual PR heads, and attach the retained manual-residue log/response evidence (redacted; no bearer material). I found no pushed attach PR/branch to bind a source verdict to yet. Load-bearing correction: the current The tracker requires StopMedia response + independent detach readback (1411 is not an automatic success), fresh post-detach enumeration, no mount_cd refresh toggle, no first-row/ambiguous-session readiness, bounded request/poll deadlines rather than a fixed 12-second/two-minute sleep, and evidence in the actual attempt/bundle. A separate recovery policy must preserve original boot failure and pre/post-cleanup power; no blanket failure=>power-off and no new destructive action smuggled into the on-or-cycle handoff. Current run 36333687404 was manually preconditioned with attached/started media. Even a successful CPU/EDAC result from it cannot establish automated detach/attach/readiness recovery. Close residue only when the implementation, real consumer, discriminating production-route control and retained unassisted execution receipt are joined. A local log plus a promise, or a named open item, is not closure. No live action was taken by this review. |
|
DECISION — APPROVE WITH AMENDMENTS: scope entry-path bare-reference admission to the entry's semantic closure, and retain the full-pool obligation in a required whole-tree phase. This is the requested decision within the pre-entry lane, discussed against still-deer-403's instrumented tree 7737ee5. It is NOT an exact-head merge approval of the unsubmitted scope change, and changes no live authorization. Please link this decision from that lane's design/implementation PR. The existing #12419 source approval is unchanged. The two intended contracts are different and both are approved:
Important precedent correction: #11092 is precedent for demand-driven EDGE PRODUCTION, but it deliberately retained the unrelated-file ambiguity refusal. Its four-module fixture made both the unrelated entry and the ambiguous consumer refuse. Removing the unrelated-entry refusal is therefore a newly authorized boundary change here, not behavior already authorized by #11092. The parse-sweep precedent supports moving an obligation to its proper consumer, but grammaticality does not discharge semantic bare-reference admission. Conditions for the implementation:
Minimum discriminating matrix:
Evidence must exercise actual entry configurations. A synthetic union closure does not establish every entry's meaning without equivalence; the #12398 coverage hold is not waived here. Ordinary source compilation is enough for this change's operational census; do not execute boot/IAM functions to measure it. Cost receipt: the proposed whole-tree selection term is replaced with reached-file work, not made physically free. Re-measure total time plus exclusive phases and admitted populations; the inclusive resolve row cannot be added to its children. Report cold and warm paths and the incremental cost in the required phase. A disappearing timer label is not itself the saved-time result. The submitted remote timings remain author-run evidence; I did not independently reproduce them. I inspected the instrumentation delta, relevant entry/model/workflow sources and #11092 history; I have not exhaustively enumerated the callers or verified a completed replacement phase. Bottom line: proceed with both independent optimizations. Step 1 preserves selection semantics; the proposed bare-reference split intentionally changes which consumer demands whole-pool admission. The latter is approved only with the full required-path cutover, scope-correct admission caching, preserved closure refusals and named consumer dispositions above. No additional strategy approval is needed for that shape; the resulting implementation still needs exact-head review. |
|
The operator's boot-process ruling and 100-host reliability target are recorded at #12423, comment 5858637192: STOP → ROOT CAUSE → strengthen the model for the failure class → exact-head review → proceed. Please route the SOL child's actual PR/head there and keep the next boot paused until its integrated deployed path clears the applicable findings; a merge event alone does not clear that hold. This does not revoke #12419's previous source approval at f9db76b or require unrelated recovery work inside this patch. The adjacent class for its review is admission/effect applicability disagreement: a new/unknown/missing mode, drift between the mode condition on the admission step and the consuming job, or skipping a refusal because the required observer input is absent. Keep the single roster authority and execution controls over the applicable refusal. No new full-tree runtime preflight is requested. Fresh get_pr_info readback reports this head non-mergeable against main; treat that as the current metadata, not the earlier queued status. Resolve and rebind source changes through the normal exact-head process. Separately, a successful non-reset skip demonstrates this optimization only; it is not evidence that media/SOL/whole-boot correctness is established. The process record explicitly separates verified functional success from safe refusal and excludes no software/transport failures by silently labeling them hardware failures. This is process coordination, not a fresh source verdict or live authorization. |
…h-landed, so the diff reduces to the drift witness Conflicts resolved per region: the workflow yml takes main's side (#12419's gate is already on main); the witness keeps this branch's stronger step-gate == job-gate assertion. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Work item adhoc-8fe41036-aa4 (operator ruling 2026-09-27: fleet-converge overhead toward 0).
Root cause. In
gunbc.fleet_converge_workflow,fleet_converge_reset_observer_dispatch_admission_stephadif_condition: none. The step's fold (host_reset_return_dispatch_admission_wet) maps every other mode toResetDispatchNotThisMode => ExitSuccess, so the mode decision was made inside agunbc runprocess that costs minutes, on every dispatch of every mode (e.g. mtcollins1_boot run 36253081549).Fix. The step now carries
fleet_converge_host_reset_return_step_if=fleet_converge_mode_step_if(mode: HostResetReturn), the same roster-derived gate the reset job's own steps use — no second list. A skip is exactly the NotThisMode arm, decided by the runner. The safety property is unchanged: forhost_reset_returnthe step still runs before the reset job is scheduled.Audit of the build job. Remaining own steps (release build, pack, upload) serve every mode: the
fleet-convergejob downloadsrelease-binsunconditionally, so none runs for a mode it doesn't serve. The checkout/prelude steps are shared prelude.Witness.
the_build_job_carries_the_reset_observer_dispatch_admissionnow also asserts the step's gate equals the HostResetReturn mode condition; removing the gate turns it red.Regenerated YAML diff: one line —
if: github.event.inputs.mode == 'host_reset_return'on the step (applied in emitter shape; the regen fixed-point lane verifies it against the model).Evidence (instrument, not transcription): before = step timings of run 36253081549, job
build, viagh run view 36253081549 --json jobs; after = the same query on the next mtcollins1_boot dispatch after merge, where the step must reportskipped. Not yet dispatched — requires merge to main (mtcollins1_boot checks out github.sha of the dispatched ref).🤖 Generated with Claude Code