Repository navigation
#13041 firmware convergence review fixes - #13096
gunbai-bot[bot] wants to merge 8 commits into
Conversation
extdeps: PICMG HPM.1 upgrade shape; Foxconn Mt. Collins release rows (measured .hpm sha256), vendor order, block sizes, BIOS/SCP pairing, BMC/BIOS interop; FirmwareTarget gains ScpFirmware and MainboardCpld. gunbc.fleet.mtcollins_firmware_baseline: §3d baseline selection via std.decision. gunbc.fleet.mtcollins_firmware_converge: re-entrant fold over gunbc.ensure with independent readback, AC cycle as a ManualStepRequired selection, credential swap after BMC reflash (sealed BmcSecureStanding + Redfish), post-set boot readback, typed refusal per failure arm, dry ipmitool realization. Witness with discriminating REDs; recurring failure mode for the account reset. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…eadback, review 74383 - floor: select_firmware_baseline names every RealizationSelectionResult arm (no wildcard) - review 74383: upload_block is ByteSize; AC-off time is std.measure Second; the Basic-Auth interop check is membership by image digest (BIOS triples do not order across families), RED: 0ACOC017 beside BMC 0.45.3 fails it - fact 3: a BMC step (ClearsSel) is not due until a SEL snapshot is read - fact 7: pre-flight refuses ES silicon the 0ACOD notes do not state, and a missing rollback image - fact 1: SCP read from SMpro register 1 through the existing SMpro probe outcome - fact 6: PostDdrProgressLost carries the agreed shape in gunbc.machine_intake_boot_outcome - CPLD step facts attributed to the observed manual run, not its unread SOP Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…and flash; whole-set pre-flight Side-chat review on ff66edd: - credential re-establishment joined to the world's subject (same_intake_subject), controller endpoint, managed account/role/SecretRef, an epoch later than the one IN FORCE (world.managed, not a hard-coded row, so a second flash cannot replay the first flash's standing), and probes after flash completion - the re-provision readback is a receipt of EVERY right the boot workflow uses (IPMI admin on LAN 1/7/8, SOL, KVM, VMEDIA, web login, Redfish), plus remote media read back by images enumerated, never by the PUT status (operator, 2026-10-03) - each executed record carries its pre-flash power reading and SEL capture, ordered inside the attempt and before the flash; a step starting before its predecessor completed refuses - one baseline-admission authority shared by selection and convergence (missing/duplicate target, pairing, Basic-Auth interop) - whole-set pre-flight before any step: complete CPU inventory, single installed/rollback reading per target, a missing BIOS rollback stops the BMC step - SCP SMpro decode carried as TranscribedUncited; the converged verdict names uncited readbacks - the MB CPLD procedure row moved downstream (observed, not vendor-stated); 0x1a0a kept opaque Review 74510: the interop requirement names the BIOS release row (its digest is owned there); image_uri derives from the delivery authority. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s (review 74564) firmware_target_eq and firmware_semantic_version_eq are deleted from extdeps.firmware.types, as are account_role_eq and managed_right_eq in the convergence and same_right in the witness: the substrate derives structural equality on coproducts and records, so the tables were a second, hand-maintained authority whose wrong cell would compile. secret_ref_same stays field-wise on purpose: SecretRef carries a hash_state that is not part of the reference's identity. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…red join (review 74585) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e CPU row per socket, steps close on timed evidence (review on 76e6f8c) - The whole-set pre-flight no longer drops out once flash records exist: the rollback check and the CPU check key on the attempt's opening readings, not on whether a record is present. - CPU completeness is one row per socket: an out-of-range socket and a socket read twice refuse. - AcPowerCycleWitness and VersionReadback carry times. A successor must start after its predecessor's step_closed_at; a start before the re-establishment evidence refuses as StepDuringFactoryCredentialWindow even when restoration succeeded later. An AC cycle witnessed before its flash completed, and a readback before its AC cycle, refuse. - Release-module prose: CPLD V06 is carried as 6.0.0, matching the row. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79041ba578
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| after: after.reading, | ||
| after_satisfies: reading_satisfies(reading: after.reading, desired: rel.version), |
There was a problem hiding this comment.
When an SCP record is supplied with hpm_check_readback(...), or an HPM component reading is attached to another target, reconciliation accepts it solely because the numeric version matches. VersionReadback carries neither a target nor a VersionReadbackRoute, and readback_route_of is not consulted here, so even the HPM route documented as unable to observe SCP can ground that step and suppress its uncited-readback warning. Carry and validate the target and route before returning EffectConverged.
Useful? React with 👍 / 👎.
| Absent => FirmwareBootReadbackDue | ||
| Present { value: o } => | ||
| match o { | ||
| HostReachedUefi => FirmwareBaselineConverged { uncited_readbacks: uncited_readbacks(records: records) } |
There was a problem hiding this comment.
After flashing, any supplied HostReachedUefi value—including one from an earlier boot or another machine—immediately marks the baseline converged. The variant carries no subject or observation time, and boot_verdict receives neither the world nor a completion bound, so it cannot prove this was the first boot after these records; a failed post-flash boot can therefore be hidden by stale success. Require a boot receipt keyed to the subject and observed after every step closed.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| fn procedure_for(t: FirmwareTarget) -> MtCollinsHpmStepProcedure? { | ||
| (filter(concat(mt_collins_hpm_procedures, [mtcollins1_observed_mb_cpld_procedure]), p => (p.firmware_release.target == t))).first() |
There was a problem hiding this comment.
When an admitted Mt. Collins baseline contains a different BMC or CPLD release—as the module expects future deliveries to do—this lookup still selects the ticket #7426 procedure solely by target. The resulting FirmwareStepDue can therefore embed a different firmware release from the baseline whose version and digest are used for decision and reconciliation. Match the procedure to the selected release identity or digest, and return NoVendorProcedure when that exact release has no procedure.
Useful? React with 👍 / 👎.
| match acc { | ||
| Present { value: _ } => acc | ||
| Absent => | ||
| match list_length(filter(b.releases, r => (r.target == t))) { |
There was a problem hiding this comment.
When a directly supplied baseline contains one row for each target but those rows name another board, this admission check still passes because none of the target, pairing, or interoperability predicates inspect FirmwareReleaseRow.board. converge_firmware can then authorize Mt. Collins procedures using a baseline whose images were declared for different hardware. Require every release in this baseline to name AmpereMtCollins before selection or convergence.
Useful? React with 👍 / 👎.
Auto-opened by session-dashboard for session
snappy-owl-38.Pushing to
session/snappy-owl-38advances this PR.Worker attestation
Before flipping this PR to ready for review, confirm each item:
npm test,cargo test) and the result.Closes #Ndirective.Summary
TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.
Test plan