Skip to content

P1AM SCADA safety batch: de-energize on fault (consolidates 11 PRs, 46 issues) - #4448

Open
dieterolson wants to merge 87 commits into
mainfrom
consolidated/p1am-scada-safety-2026-08-13
Open

P1AM SCADA safety batch: de-energize on fault (consolidates 11 PRs, 46 issues)#4448
dieterolson wants to merge 87 commits into
mainfrom
consolidated/p1am-scada-safety-2026-08-13

Conversation

@dieterolson

@dieterolson dieterolson commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Consolidates the P1AM SCADA production-readiness remediation for the Raspberry Pi 5 gasifier control system. The original review's finding was that nothing de-energizes on fault; this branch is the batch that fixes that.

Supersedes #4045, #4053, #4057, #4058, #4059, #4060, #4062, #4064, #4066, #4067, #4068

Two of the thirteen candidate PRs are deliberately NOT merged — #3959 and #4043 — because each introduces a safety inversion. Details and required rework are in the verdict table and the section below. They stay open.

Per-PR verdict

PR Scope Verdict Notes
#4060 E-stop / shutdown safe state MERGED E-stop opens HEATER_RELAY_COIL first and hard-errors on a missing ack; the old 64-register TAG_VALUE_BASE write (a provable no-op the firmware overwrites every scan) is gone; shutdown safes the plant before joining tasks, on the exception path too, under a deadline below TimeoutStopSec. Dead write seams now raise instead of returning True.
#4059 Units contract / sensor faults MERGED Converts the power-supply thermocouple from percent-of-full-scale to degC, which made HH_TEMP — a degC threshold — reachable at all. Absent/non-finite feedback latches SENSOR_FAULT instead of substituting 0.0. Deglitch filters are built per channel so an open TC is no longer accepted as a real reading.
#4045 PID / MPC control math MERGED Verified the maths independently: the 28.3 %/63.2 % two-point identification is correct (t28 = θ + τ/3, t63 = θ + τ ⇒ τ = 1.5(t63 − t28)), and the DMC change to solving for Δu from a zero origin is right — free_response already carries the effect of holding last_cv, so optimising over absolute CV double-counted the current input. Stops reporting a clamped-to-zero open-loop controller as status="success".
#4064 Poll-loop integrity / cadence MERGED Splits trusted_tags (control, alarms, historian) from display_tags (HMI only); a failed read yields HELD/FAULT and the alarm engine is not run, so a link flap can no longer clear an active HiHi. Scan/trip cadence pinned to poll_interval_s — previously the default LIGHTWEIGHT mode ran the whole loop at 2.0 s, making E-stop re-assert 20× slower. Bounded queues; backpressure drops samples but never alarm events.
#4058 Historian retention / UTC / bounds MERGED Fixes a genuine P0: the retention sweep ran synchronous SQLite (including a whole-file VACUUM) on the event loop, freezing the control loop, the broadcast and every HTTP endpoint including E-stop. Now asyncio.to_thread + bounded incremental_vacuum. Also fixes UTC drift via a UtcDateTime decorator and bounds Data Explorer memory by row count before reading rows.
#4066 Deployment hardening MERGED with fix Auth audit found no fail-open path (503 when unconfigured, closed-set validation, require_read_auth defaults true). Its final commit had deleted 24 authz-matrix rows as "stale"; 14 of those routes are mounted unconditionally, so test_every_route_is_classified failed on the merged tree. Restored — see below.
#4053 Web HMI data age / alarm loss MERGED Replaces a boolean liveness flag with a real data age, and stops one malformed alarm suppressing the whole active-alarm list. Caveats filed as follow-ups, not blockers.
#4057 Desktop HMI annunciation MERGED with 2 fixes Correct HH/LL from configured limits, inclusive comparisons (conservative direction), E-stop trip immediate and unguarded while only clearing is admin-gated. Two defects fixed — see below.
#4062 Calibration shutdown / alarm ack / MFCs MERGED with fix cmd_teardown commands AOs to zero and reads them back before releasing routing, and main() zeroes outputs on SystemExit/KeyboardInterrupt. Ack correctly never clears a still-active condition. One blocking defect fixed — see below.
#4067 Frontend localStorage test setup MERGED Correct root-cause analysis of vitest 1.6 populateGlobal. Combined with the competing implementation already on the branch — see conflicts.
#4068 Historian DB path anchoring MERGED Anchors the SQLite historian to the package dir with a P1AM_DB_PATH override, as_posix() so a Windows drive path does not become escapes, and unanchored .gitignore patterns. Well tested.
#3959 Heater bring-up + 4-source TC input EXCLUDED The new analog thermocouple path fails LOW. Firmware clamps sub-4 mA to 0 %, which with the affine defaults reads −150 °C (analog K) / +65 °C (analog R) — maximally "cold", i.e. call for full heat. None of the three claimed mitigations catches it: the deglitch filter is constructed with no arguments so it keeps the 1400 °C/5 °C rails (analog R's 65 °C floor can never trip burnout_low), the first sample after a restart is accepted as good, and the cross-check now compares two channels sharing one supply, conditioner family and analog card, so TC_DISAGREE cannot fire. The PR's own tests encode this: test_controls_on_analog_k_tag feeds the broken-loop signature and asserts relay_on is True, and test_thm_other_sensor_not_used_on_analog_path asserts HH_TEMP not in trips with the card R at 1400 °C. Separately, SignalBroker::kNumInputs goes 6 → 8, widening ConfigStruct by 8 bytes, without bumping StorageManager::kMagic — the file's own comment states that rule; a retained config would load misaligned and publish AI2/AI3 onto the AO command tags. Also counts * (20.0/8191.0) assumes 0 counts = 0 mA while the module README describes a 4–20 mA range; if wrong it under-reads by ~190 °C, and the bench evidence cannot distinguish the two mappings. Needs rework, not merging.
#4043 Firmware harness + comms watchdog EXCLUDED Commit 1 (harness repair + CI gate) is good. Commit 2 inverts the interlock in both directions against the shipped config. defaults.py ships lolo=0.0, low=5.0, high=95.0, hihi=100.0 for all 32 tags and firmware.ino copies regs 300..555 into the interlock every scan. With IsLimitEffective accepting the closed interval (limit >= 0 && limit <= 100 — its own test asserts IsLimitEffective(0.0f) and (100.0f) are true) and the new trip being val <= lolo, any idle tag sitting at exactly 0.0 trips on the first scan; the trip action itself forces routed output tags to 0.0, so ClearTrip() can never stick — a permanent, unclearable latch, which is precisely the #4001 symptom the commit claims to fix. Meanwhile hihi = 100.0 sits on SetTag's clamp ceiling, so the over-temperature trip that previously fired at high = 95.0 (≈1330 °C) becomes unreachable. Verified against the working tree, not inferred. Land commit 1 separately after pinning the arduino-cli installer (currently curl … master/install.sh | sh on a self-hosted runner from a pull_request trigger) and gating the compile job against fork PRs.

Defects fixed during integration

Four fail-open defects, three of which defeat the guarantee their own slice was written to provide. All four have regression tests.

  1. A fabricated reading could clear a live alarm (backend/state.py). _restore_engine_alarms called update_tag(tag, float(entry.get("value", 0.0))). get_active_alarms() is only contracted to agree between the Rust tools_core.scada engine and the Python fallback on the keys the restore path reads — not on carrying value at all — and where it is absent that substituted 0.0 resolved the tag to Normal. apply_config clears active_alarms first and runs on every routing deploy and every reconnect-time _publish_active_config. Composed with SCADA: fix poll-loop data integrity, cadence and backpressure (#4004 #4008 #4009 #4023 #4024) #4064 this is worse than either alone: the poll loop then refuses to re-evaluate alarms while the link is down, so a HiHi cleared by the reconnect stays cleared for the whole outage. A record with no finite value now retains its snapshotted state and never touches the engine.
  2. NaN was classified as "normal" (desktop/alarm_state.py). classify_value decides the band with four comparisons and every comparison against NaN is False, so NaN fell through to return None = inside the normal band, and evaluate emitted cleared and dropped the tag from both sets — silencing a live High-High on the heater. json.loads accepts bare NaN, so it was reachable off the wire. _require_number now rejects non-finite input and _evaluate_alarms catches it alongside TypeError; skipping the tag is fail-safe because evaluate validates before it clears anything.
  3. The new annunciator flashed at 10 Hz into a steady light (desktop/header.py). set_alarms_state unconditionally set _flash_state = True and _refresh_annunciator calls it on every telemetry frame, so the timer's OFF phase was overwritten within ~100 ms. The phase now restarts only on a transition into unacknowledged.
  4. The authz exhaustiveness net had a hole punched in it (test_route_authz_matrix.py). 14 restored rows are the DC supply's and heater's own command surface. The 9 Data Explorer rows genuinely are conditional (that router mounts only when its numeric stack imports), so they now register from the live app rather than being deleted — satisfying both the exhaustiveness and no-stale-rows checks in either environment. 51 tests with 1 failure → 84 passing.

Plus one broken test repaired rather than deleted: test_connect_loop_wakes_immediately_on_shutdown reused the import-time module-level shutdown_event, which asyncio binds to a loop on first await, so it raised "bound to a different event loop" before reaching its assertion.

Two further defects surfaced by CI, one of them serious

Both were found while driving tests (3.11) green. In each case the cheap fix would have cemented a real defect.

1. A safety event log could show a trip above its own acknowledgement

This is the more serious of the two. EventLogger.fetch_logs ordered by timestamp DESC alone. datetime.now() reports microseconds, but the underlying clock does not — ~15.6 ms granularity on Windows — so a burst of events (an alarm trip and the acknowledgement that answers it; a setpoint change and the trip it causes) share one timestamp string, and SQLite is then free to return them in any order.

The visible symptom was a flaky test. The actual consequence is that an operator reviewing the event log could see an alarm trip listed above the acknowledgement that answered it — misleading evidence in a safety record, during exactly the review where ordering carries the meaning.

Fixed with ORDER BY timestamp DESC, id DESC, using the existing autoincrement primary key, which is monotonic in insertion order. test_fetch_logs_orders_same_timestamp_events_newest_first pins it by writing an explicit identical timestamp so the tie is forced rather than hoped for; verified as a real regression test, since with the tiebreaker removed it fails and returns ['first', 'second', 'third'].

Note on provenance: the test that exposed this (tests/p1am_control_system/test_desktop.py::test_event_logger_basic) is byte-identical to main — this branch never touched it. The flake is pre-existing on main and only surfaced here because event_logger.py is in this diff, so the source-keyed selection ran it. It failed roughly one local run in three.

2. The authz matrix disagreed with itself about 19 routes

test_table_has_no_stale_rows reported all 19 power-supply / temperature / tuning rows as unserved. They are served, proven from inside the same failing run: single process (-n 0), the 19 matching test_gated_route_rejects_anonymous_caller cases drove those exact paths through TestClient(app) — the same module-level app the enumeration walks — and got 401/403. _DENIED = (401, 403), and a route the app does not serve answers 404, which would have failed.

Deleting the rows (which an earlier slice had done, and which this PR reverted) would have made the check green while the authz matrix quietly agreed that acknowledge_trip, permissive, setpoint, burnout_mode, tc_type and the whole PID tuning lifecycle did not exist — the plant's entire control surface, in the batch whose review theme is "nothing de-energizes".

test_table_has_no_stale_rows now asks Route.matches(), the function Starlette itself calls during dispatch, so a check phrased as "would a request reach a handler" cannot disagree with a request. Verified it still discriminates: real and parameterised routes match, a bogus path does not, a wrong method on a real path does not.

The underlying divergence is not root-caused and is not reproducible locally (CI's exact 101-file selection with CI's exact flags passes 1949/1949, three times). Filed as #4476, including the part that matters most: test_every_route_is_classified is deliberately left enumeration-based because it guards the ungated direction, which means it is still exposed to the same under-reporting — a route the enumerator misses is a route it can never demand an authz row for. Latent today (every current route is classified and gated with passing enforcement tests); the risk is the next route added. Not fixed here, by design.

Conflicts resolved, and which side won

18 conflicts across 11 merges.

Also removed from the merged result

  • ~290 files of formatting churn. Each slice ends with "pre-commit automated fixes" commits produced by an older pinned ruff than the ruff==0.14.10 CI and .pre-commit-config.yaml now use. 293 Python files outside the P1AM set are restored to origin/main exactly; 256 were provably pure formatting, the rest sit in ruff-excluded trees main has never had formatted. Left in, each would have been a quality-gate format failure for a change nobody made.
  • Six .codex-worktrees/* gitlinks (git mode 160000). No .gitmodules declares them, origin/main has none, and the commits they name exist on no remote — a fresh clone would carry six gitlinks pointing at objects the server does not have.
  • CRLF in nine files where main uses LF. Not cosmetic: deploy/launch-hmi.sh would fail at exec on the Pi with bad interpreter: /bin/bash^M, so the operator's HMI launcher simply would not start; backend/Dockerfile was also converted. Only *.py is covered by .gitattributes.

Gates run locally

Issues carried by this PR (46)

#3996, #3997, #3998, #3999, #4000, #4003, #4004, #4005, #4006, #4007, #4008, #4009, #4010, #4011, #4012, #4013, #4014, #4015, #4016, #4017, #4018, #4019, #4020, #4021, #4022, #4023, #4024, #4025, #4026, #4027, #4028, #4029, #4030, #4031, #4032, #4033, #4034, #4035, #4036, #4037, #4038, #4039, #4040, #4041, #4042, #4061

Not carried: #3995 (firmware never compiled or tested in CI) stays open — it was #4043's, which is excluded. #4001 and #4002 are already closed, and #4043's commit 2 would have re-opened #4001 in practice.

Follow-ups (reviewed, deliberately out of scope)

Real findings that are pre-existing or non-blocking, kept out to avoid unreviewed behaviour change on a safety branch:

  • _resolve_source in the poll loop keys provenance on None only, so a dict containing NaN still classifies as LIVE. SCADA: fix poll-loop data integrity, cadence and backpressure (#4004 #4008 #4009 #4023 #4024) #4064 closes the None case, not this one.
  • plc.read_tags() raising (rather than returning None) unwinds _poll_once before the write-seam interlocks are armed and before any DATA_QUALITY event is emitted.
  • A partial read renders absent tags as 0.0 stamped live on the display path.
  • deploy/launch-hmi.sh passes the operator credential in --app=…#apikey=…, i.e. in /proc/<pid>/cmdline, readable by any local account.
  • AuditMiddleware._buffer_body bounds only the declared content-length, so a chunked body is unbounded pre-auth.
  • fix(p1am-hmi): report data age, not a boolean; stop silent alarm loss #4053's snapshot path re-stamps liveness from a cached frame, so a poll loop that dies after one good poll can hold the age at 0; AlarmsHeader downgrades critical to warning when any entry is dropped; TemperatureConfigPanel carries an unfixed parseFloat("") → NaN into hh_limit_c.
  • Two contradictory driver classifiers (REAL_PLC_DRIVERS treats unknown as simulated; SIMULATED_PLC_DRIVERS treats unknown as real hardware) should be one predicate.
  • Physical Alicat MFC transport is still not implemented (update_setpoint returns False); fail-safe, but the gas path is non-functional on the real rig.

🤖 Generated with Claude Code

dieterolson and others added 30 commits July 31, 2026 17:40
@dieterolson @claude
fix(p1am): stop reporting untrustworthy PID tunings and fix MPC move …
9b32479 
…formulation

PID tuning (#4018):

- as_response() clamped every recommended gain to >= 0. A reverse-acting
  process (identified Kp < 0) legitimately tunes to negative Cohen-Coon
  gains, so the clamp produced kp=ki=kd=0 and still reported "Tuning
  parameters identified successfully" - an open-loop controller presented
  as tuned. Gains now keep their sign and a reverse-acting identification
  is reported as a warning naming the required loop configuration.
- Dead time was floored at MIN_TIME_PARAM with no noise rejection and no
  minimum-sample check. Because Kc scales with tau/theta, a first crossing
  landing on the step sample inflated the recommendation by more than an
  order of magnitude and returned it as success. The identification is now
  rejected when the first crossing falls within two sample intervals of the
  step or both thresholds land on the same sample, and downgraded to a
  warning when theta hits the floor, when the dead-time ratio leaves the
  Cohen-Coon validity band, or when a gain exceeds the sanity bound.
- Two-point identification now uses the published 28.3%/63.2% pair with
  tau = 1.5*(t63 - t28) and theta = t63 - tau. The old 10% pair biased
  theta high and tau low by roughly 0.105*tau.

The Cohen-Coon coefficient formulas were verified against Cohen & Coon
(1953) and left unchanged.

MPC (#4039):

- The DMC solver initialised its decision variable to the absolute CV and
  then multiplied it by the step-response dynamic matrix as if it were a
  move vector, while free_response already carried the full effect of
  holding that CV. The current input was counted twice, leaving the
  /api/mpc/simulate comparison chart with a large permanent offset at any
  nonzero operating point. The solver is extracted into solve_dmc_move(),
  now solves for delta-u from a zero origin, bounds the moves, and the
  caller integrates and clamps to the 0-100% output range.

Refs #4033: the clamp test was vacuous (step_triggered=False made the
gains 0.0 by construction) and is replaced by tests that build a
TuningResult with genuinely negative gains and assert the new contract.

test_backend_p1am_safety.py: the fixed-history tuning test encoded the
biased method - its PV values 11.0 and 16.32 were exactly the old 10% and
63.2% thresholds, recorded at a 1 s sample interval with two points on the
transient, which the corrected identification cannot resolve. The same
plant (Kp=1, tau=1, theta=0.5) is now sampled at 0.1 s, and the old
under-sampled recording is retained as a rejection fixture so the guard
itself is covered.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dieterolson @claude
fix(p1am-hmi): correct alarm annunciation, thresholds, E-stop gate an…
233e988 
…d log churn

Closes #4012, #4019, #4021, #4022.

- Alarm annunciation now separates "condition present" (colour) from
  "acknowledged" (flash vs steady). Acking a still-active alarm no longer hides
  it for the session, and a cleared alarm is dropped from both sets so the ACK
  button stops flashing. Acknowledge applies only to the alarms the header was
  displaying. Extracted into desktop/alarm_state.py.
- HH/LL severity uses the deployed hihi_limit/lolo_limit rather than
  high_limit +/- 5, so HMI severity matches the firmware trip points. Interlock
  ordering is validated at config load and a bad set is rejected loudly.
- Connection status is derived from the telemetry frame
  (desktop/connection_state.py); a live plant is no longer labelled
  "Simulating" forever.
- Clearing the E-stop is Admin-gated and modally confirmed via the shared
  desktop/guards.py helpers; a declined clear latches the button back to
  tripped.
- Alarm events are debounced/counted, written in batches on a background thread
  over one persistent SQLite connection, the History table is requeried only
  while visible, and the event table has a retention window.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dieterolson @claude
fix(p1am-hmi): report data age, not a boolean; stop silent alarm loss
056d361 
The operator HMI could tell an operator a dead process was healthy, and
could command hardware from values the operator never entered.

#4010 — every field of telemetryFrameSchema is optional, so `{}` parsed
successfully and applyFrame stamped it as a live frame. The backend's
latest_frame starts as {} and is only reassigned on a successful poll,
so a dead poll loop left the HMI green forever with the trend appending
a frozen value. Liveness now requires a recognised payload field, is
expressed as a data AGE, and stale process values are greyed and
cross-hatched so frozen is distinguishable from steady.

#4011 — .catch sat on the whole active_alarms record, so one malformed
alarm erased the entire map and the header kept saying "All normal — no
active alarms" while alarms fired on the PLC. Resilience is now per
entry, dropped entries raise a degraded-data banner, and
/api/alarms/active is reconciled periodically so the list has a recovery
path independent of the stream.

#4013 — the Alicat setpoint draft was re-seeded from live telemetry on
essentially every frame, so it fought each keystroke and Set could
commit a partially-reverted value. The entry is operator-owned from the
first keystroke; the device setpoint is now its own readout.

#4020 — the power-supply staged setpoint was never seeded and +/-
commanded immediately, so one tap after a kiosk reload could collapse a
30 A output by 29 A. It seeds from the live setpoint and +/- stage only;
Apply is the sole write path, as the file's contract already stated.

#4042 — the event log is fetched on mount and refreshed on an interval
instead of only after an unrelated acknowledgement; approaching-alarm
cues come from the server-enforced config rather than the local draft,
and config entry rejects non-finite input that used to store NaN and
silently switch the pre-alarm cue off.

#3996 — none of the frontend's lint, build or test scripts were run by
any workflow. Adds .github/workflows/p1am-frontend.yml. Making that gate
useful also required repairing the pre-existing suite: Node >= 22 ships
its own localStorage global, which blocks vitest's jsdom Storage and made
15 layout-persistence tests fail with "localStorage.clear is not a
function".

schemas.test.ts previously asserted the #4011 alarm suppression as
correct behaviour; that test is rewritten. The tuning tab is extracted
to TuningPanel.tsx to bring App.tsx back under the 1500-line budget.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dieterolson @claude
fix(p1am/historian): unblock E-stop during retention, fix UTC drift, …
beae5b8 
…bound explorer loads

Closes #4006, #4025, #4026, #4027, #4040. Refs #4033.

- #4006 (P0): historian_retention_loop is an asyncio task, so its synchronous
  SQLite body ran ON the event loop, freezing the control loop, the websocket
  broadcast and every HTTP endpoint including E-stop for the length of a VACUUM
  (tens of seconds on a 1 GiB DB on SD). The sweep now runs via
  asyncio.to_thread, and reclaims disk in bounded incremental_vacuum chunks
  instead of a whole-file VACUUM. Legacy DBs are converted to
  auto_vacuum=INCREMENTAL once at startup, before the controller is live.

- #4025: TagLog/EventLog.timestamp now use a UtcDateTime TypeDecorator, so
  SQLite can no longer discard tzinfo on write nor hand back naive datetimes on
  read. Every isoformat() on the API boundary emits an explicit offset.

- #4026: _load_historian decides its cell budget from row COUNTs BEFORE reading
  any rows, and honours HistorianSource.max_points (previously dead) by reusing
  query_trend_series' streamed decimation, so peak memory tracks output size.

- #4027: the size cap is split into separately-tracked TagLog and EventLog
  budgets, each charged against its own dbstat footprint, and EventLog gains
  age-based retention. A fat event log can no longer inflate bytes-per-row and
  erase the tag historian.

- #4040: export column alignment is validated unconditionally, so ragged
  columns are a 400 instead of a truncated CSV body behind an HTTP 200.

- #4033: the index-migration test now calls database._migrate_historian_indexes
  instead of re-executing a hand-copied duplicate of its SQL.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dieterolson @claude
fix(p1am): de-energize on E-stop and on shutdown; kill dead write seams
b6696b6 
An E-stop never opened the heater relay, and half of what it did write was a
provable no-op. Shutdown never de-energized anything and blocked past the
systemd kill timeout. Direct tag writes reported success for writes the
firmware overwrites within one scan.

- trigger_estop opens HEATER_RELAY_COIL first and treats a failed ack as a
  hard error; the 64-register TAG_VALUE_BASE block write is removed (the
  firmware republishes 0..63 from its broker every scan and never reads them
  back). POST /api/estop reports success only on an acknowledged de-energize.
- lifespan teardown drives the plant safe before joining tasks or closing
  sockets, on the exception path too, under a deadline below TimeoutStopSec.
  The PLC-connect retry waits on shutdown_event instead of sleeping past it.
- write_tag raises NotImplementedError for the republished TAG_n block instead
  of returning True; the PID tuner steps through write_pid_setpoint and skips
  identification unless the step was acknowledged.
- write_tag and write_routing now honour the defense-in-depth E-stop latch; a
  contract test fails if a new write seam escapes the latch registry.
- New write_heartbeat seam for the firmware liveness watchdog (register 560).
  Per-scan call site still needs wiring by the poll-loop PR.
- E-stop tests assert addresses and values, with a fake recording both
  write_registers and write_coil, instead of a write count.

Closes #4000
Closes #4005
Closes #4015
Closes #4038
Refs #4033
Refs #3999

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dieterolson @claude
🐛 P1AM SCADA: safe calibration shutdown, real alarm ack, settings-dri…
f0eb448 
…ven MFCs

Fixes three P1 defects on the P1AM control system.

#3997 — calibration left analog outputs energized. teardown now commands each
pass-through PID to 0.0, reads the AO tag back to CONFIRM 0%, then unmaps the
PIDs and releases the output routing so the firmware's own WriteAnalogOutput(i,
0.0f) safe path takes over. main() drives the AOs to 0% on any abnormal exit
(exception, SystemExit, KeyboardInterrupt) before plc.close(), logging loudly
without masking the original cause.

#4034 — alarm acknowledgement never reached the alarm engine. The endpoint now
forwards to AlarmEngine.acknowledge_alarm(tag_id, user) and populates
acknowledged_by; apply_config migrates alarm state (including the ack) into the
rebuilt engine through the public API shared by the Rust and Python engines, so
an ack survives a routing deploy and a PLC reconnect.

#4031 — every mass flow controller was hardcoded to simulation. The transport
now comes from P1AM_ALICAT_CONNECTION_TYPE / P1AM_ALICAT_PORT_OR_IP; mock
devices are refused whenever plc_driver drives real hardware (gas control is
then absent, with registration_error set and a CRITICAL log, rather than
silently simulated). AlicatMFC validates its transport and port, and
parse_ascii_response no longer bypasses the VALID_GASES check.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dieterolson @claude
🛡️ SCADA: fix poll-loop data integrity, cadence and backpressure
0a83e6a 
- #4004: classify each scan's provenance (live/simulated/held/fault). Held
  and faulted values never reach the control laws, the alarm engine or the
  historian, so a link flap can no longer clear an active HiHi. The backup
  simulator is only wired into the scan path on a simulator driver. TagLog
  gains a `quality` column (with migration) so an outage is a gap, not
  fabricated continuity. Frames carry data_source/plc_connected/simulated,
  and a live scan strokes the firmware heartbeat (#4044).
- #4008: pin the scan/alarm/relay/E-stop cadence to settings.poll_interval_s;
  the performance mode now only decimates the WebSocket broadcast.
- #4009: schedule against a monotonic deadline with overrun counting and
  phase resync; size the Modbus timeout to the scan period; compute the
  failure backoff from the active period.
- #4023: batched, retrying historian writer on a worker thread.
- #4024: per-client bounded WS queues, serialise once, drop oldest.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dieterolson @claude
🔒 Harden P1AM SCADA deployment: the authz was fine, the deployment wa…
5ece271 
…sn't

The in-source authorization was already correct — every hardware-mutating route
carried require_admin_key, E-stop clear was admin-gated, the WebSocket was
authenticated, comparisons used hmac.compare_digest, and cors_config.py failed
closed. Every exploitable defect was in the deployment, or in the client's
inability to authenticate.

#4007 (P0) — production installs shipped with auth disabled, and the HMI
re-exposed the loopback-bound backend. Fixed as one change, because fixing any
part alone ships something either exposed or unusable:
  - the HMI can now authenticate (frontend/src/api/credentials.ts): X-API-Key on
    every request, the key sent as the FIRST WebSocket frame rather than a query
    parameter, stored per browser profile, seeded by the kiosk launcher through
    a URL fragment it strips on load;
  - install-services.sh no longer bakes P1AM_DEV_NO_AUTH=1 into the unit. It
    generates random operator/admin keys into a root-owned EnvironmentFile,
    preserves them across re-runs, refuses to write a unit without one, and
    gates the bypass behind an explicit --bench flag;
  - vite preview binds 127.0.0.1 instead of every interface.

#4041 — the credential tiers are now nested. A configured admin key satisfies
the operator tier, so an admin-only deployment no longer means full hardware
control behind a dead display. The resolved posture is logged at boot.

#4037 — require_read_auth defaults on and is attached to the ungated read
routes (/api/routing disclosed the whole register map). RequestGuardMiddleware
refuses state-changing requests from a non-allowlisted Origin and forces a
preflight, closing the bodyless CORS-simple POST path that let any page the
kiosk opened command the plant. The panic stop stays curl-reachable.

#4029 — an append-only AuditEvent table plus one ASGI middleware, so a NEW
endpoint is audited by default. Records route, redacted payload, credential
tier, non-reversible key fingerprint, client IP and status; unreachable from
the client-writable POST /api/events and untouched by /api/capture/clear, so
the trail can be neither forged nor erased. Mirrored to journald.

#4028 — test_route_authz_matrix.py boots the real app with keys set and the
bypass cleared, driving an explicit (method, path, tier) table. An unclassified
route fails the suite, so a new endpoint cannot ship ungated.

#4014/#4030/#4036 — the deployment can actually work: a `p1am` extra is the
single source of truth for backend runtime deps (adding the missing
pydantic-settings and python-multipart), mirrored as exact pins in the
Dockerfile with drift caught by a test; the container binds 0.0.0.0 and is
isolated at the publish layer; compose uses the env-var names settings.py
reads and mounts the historian at /data, not over the source; the HMI is built
at install time and both units carry Nice=/CPUWeight=; the numpy lock no longer
contradicts requirements.txt; and PLCFactory logs an unmissable banner when the
simulator is driving the HMI's "live" values.

Also fixes #4061: several suites mutated os.environ at module import time and
one popped P1AM_DEV_NO_AUTH there, so whichever module was collected last
decided whether seven endpoint tests passed or 503'd. Those are now per-test
monkeypatch fixtures. Verified green in both collection orders.

Closes #4007
Closes #4014
Closes #4028
Closes #4029
Closes #4030
Closes #4036
Closes #4037
Closes #4041

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dieterolson @claude
fix(p1am): anchor historian DB to package dir; ignore stray dcs_scada.db
1ff5a7f 
`database.DATABASE_URL` was a bare relative `sqlite:///dcs_scada.db`, so the
SQLite file resolved against the process CWD rather than a fixed location. Two
consequences:

1. The historian silently forked into a different DB per launch directory. A
   `uvicorn main:app` from `backend/`, a test run from the repo root, and a
   systemd unit with its own WorkingDirectory each got their own file, so tag
   history appeared to vanish depending on how the backend was started.
2. `pytest src/p1am_control_system/backend/tests/` from the repo root dropped an
   untracked `dcs_scada.db` at the repo root, which `git add -A` would stage.
   The .gitignore entries were path-anchored to the backend directory and did
   not cover it.

Fixes:

- `DB_FILE` now resolves to an absolute path anchored to the backend package
  directory via `Path(__file__).resolve().parent`, with a `P1AM_DB_PATH` env
  override for deployments keeping the historian on separate storage. This
  makes the location documented in BENCH_HANDOFF.md (`backend/dcs_scada.db`)
  authoritative. The URL uses `as_posix()` so a Windows drive path does not put
  backslashes into `sqlite:///...`, where they would read as escapes.
- Docker is unaffected: the image does `WORKDIR /app` + `COPY . /app`, so the
  package directory *is* `/app` -- the same path the compose `dcs_db_data`
  volume mounts. The default resolves where it always did.
- Also fixes `data_capture._db_size_bytes`, which called
  `os.path.getsize(DB_FILE + suffix)` and was therefore measuring whichever DB
  happened to sit in the CWD (or nothing), making the historian size cap and
  status display inaccurate.
- .gitignore entries are now unanchored (no slash => git matches at any depth),
  covering both the backend directory and any other CWD.

Verified: full p1am backend suite passes from the repo root (878 passed, 6
skipped), no stray DB at the root afterwards, ruff check + format clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dieterolson @claude
fix(p1am-frontend): install conforming localStorage in test setup
885c1a1 
15 tests across panelLayout.test.ts, usePanelLayout.test.ts and
PanelStack.test.tsx failed with "TypeError: localStorage.clear is not a
function".

Root cause is in vitest 1.6's populateGlobal, which only copies a jsdom
global over an already-present Node global when the key is in its own
curated KEYS list:

    if (k in global) return keysArray.includes(k);

Neither localStorage nor sessionStorage appears in that list, and Node
22+ defines both on globalThis, so jsdom's Storage is never installed
and Node's wins. Node's sessionStorage is in-memory and behaves, which
is why only localStorage broke; its localStorage is backed by
--localstorage-file and, with no valid path, degrades to a bare object
with none of the Storage methods.

jsdom's own localStorage works fine here, it is simply unreachable, so
pinning the environment is not a fix: vitest 1.6 exposes no lever to add
keys to that list. Install a spec-shaped in-memory Storage in the shared
test setup instead, which fixes all three suites at once and any future
one.

Installed unconditionally rather than behind a conformance guard:
setup.ts only runs under the test runner, a fresh Storage per test file
is the isolation we want, and merely reading Node's lazy localStorage to
probe it fires a --localstorage-file warning in every worker.

Verified: 44 files / 381 tests pass (was 366 passed, 15 failed), tsc
--noEmit clean, eslint 0 errors.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dieterolson @claude
test(p1am): configure auth per test, not at import time (#4061)
c8d1407 
The two test modules this branch introduced set P1AM_DEV_NO_AUTH by mutating
os.environ at import time. settings is a module-level singleton read once at
first import, so whether the variable lands before or after that import depends
on collection order and xdist worker assignment.

Concretely, in the combined run
`pytest src/p1am_control_system/backend/tests/ tests/p1am_control_system/`,
tests/p1am_control_system/test_backend_security.py imports last and pops the
variable, so TestTunerStepWritePath fails with 503. Reverse the two path
arguments and the same tests pass. Order-dependence in the direction of
passing is the dangerous direction over an E-stop write path.

- test_estop_endpoint_writes.py: import-time assignment replaced with an
  autouse `_bench_no_auth(monkeypatch)` fixture, matching the convention
  PR #4066 established for the pre-existing instances. Its TestClient also
  carries the X-Requested-With HMI marker that PR's request guard expects
  (inert here, avoids a second convention).
- test_estop_shutdown_safe_state.py: assignment removed outright. The module
  drives the lifespan and the safe-state seams directly and issues no HTTP
  request, so it needs no credential posture — and its import-time mutation was
  leaking into every suite collected after it.

Refs #4061

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@dieterolson
fix(p1am): scale the power-supply thermocouple, fault on missing feed…
8b495b2 
…back

Four defects in the power-supply and temperature sensor paths, each with a
failing test written first.

#4003 -- the firmware publishes thermocouples as PERCENT of full scale, and
_inputs_from_tags scaled current and voltage but passed the temperature tag
through as though it were already degC. HH_TEMP compares that against a degC
threshold (default 1200), and broker tags are bounded [0, 100], so the power
supply's only over-temperature interlock could not fire under ANY physically
possible reading. The HMI showed "86 degC" on a load at 1200 degC, so the
automatic protection and the human backstop were reading the same wrong number.

#3998 -- full scale was declared in three places (firmware, temperature_models,
thermocouple_filter) with a comment asking them to agree. It is now one
definition in hardware.py, the module that already owns the firmware contract,
with percent_to_celsius/celsius_to_percent as the only conversion. A new test
parses the firmware source and fails if the two halves drift, so the contract is
a gate rather than a comment.

#4016 -- any absent tag became 0.0 via dict.get, and non-finite readings were
coerced to 0.0. Unlike the temperature controller there was no sensor-fault
state, so a renamed tag or a pulled monitor lead silently zeroed both trip
inputs while the output stayed energized, and reported a confident, cold-looking
supply. Missing or unusable feedback now raises SensorFeedbackError and latches
a new SENSOR_FAULT trip; a genuine zero is still a reading.

#4017 -- set_current_setpoint/set_power_setpoint returned the clamped request on
every path including the IDLE and TRIPPED branches that discard it, and the
service persisted the raw request regardless. An operator commanding 40 A into a
latched controller got 200 {"applied_a": 40.0} and went troubleshooting the load
instead of the trip; the bogus value was also pre-filled after the next restart.
Both setters now return what is in effect, and persistence is gated on the
command actually taking.

#4035 -- both deglitch filters were constructed bare, pinning them to the 1400 C
default whatever the channel was configured for. On a shorter-range channel the
high-side burnout rail sat above any reachable reading, so an open thermocouple
was accepted as genuine -- the exact condition the filter exists to catch. They
are now built per channel with max_step_c scaled to the span, and rebuilt when
the config changes so a rescale cannot strand filter state and trip TC_FAULT
mid-run.

Three existing tests encoded the defects and are updated to the corrected
contract: the units pass-through assertion (#4003), IDLE echoing the request
back (#4017), and "no tags uses zero feedback" (#4016).

Also gitignores a root-level dcs_scada.db -- the suite creates the historian
relative to the working directory, so a run from the repo root drops a stray
artifact that is easy to commit by accident.

894 passed, 6 skipped in the backend suite.

Refs #4003 #3998 #4016 #4017 #4035
@dieterolson @claude
refactor(p1am-hmi): split TemperatureControl; fold recall onto seedDr…
08427b1 
…aftText (#4063)

TemperatureControl.tsx was 1975 lines — 475 over DEFAULT_MAX_SOURCE_LINES —
so fleet-fast-guardrails rejected any commit touching it. The heater screen
was the one operator surface that could not be corrected without first being
restructured.

Following the TuningPanel shape (#4053), the container keeps all state, the
rolling trend buffer and every /api/temperature/* call; the sections are
prop-driven components:

  TemperatureControl.tsx      1975 -> 717
  TemperatureTrend.tsx             680  (SVG trend + its own view state)
  ThermocoupleSelector.tsx         196
  TemperatureConfigPanel.tsx       148
  TemperatureStatusHeader.tsx      136
  HeaterStartStopButton.tsx         55
  lib/temperatureTrend.ts          171  (pure sample/readout math)

The Start/Stop command button existed as two byte-identical copies on the same
screen; on a control that energizes a heater those could come to disagree about
whether a command is safe to send. One component now serves both, differing
only by an appended CSS class.

DRY cleanup #4053 had to defer: recallSetpointText now returns
seedDraftText(lastSetpointC, operatorTouched, 1) rather than carrying its own
copy of the operator-ownership rule. The heater domain types moved to
src/types.ts, removing the cycle where useTelemetryStream imported
TemperatureStatus from a component.

No behaviour change: every moved block is verbatim apart from prop renames and
JSX whitespace that collapses identically. lint (0 errors, same 2 pre-existing
warnings), build and 444/444 vitest tests pass.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@dieterolson
Merge branch 'main' into fix/p1am-db-path-anchoring
26ed24a
@dieterolson
Merge branch 'main' into fix/frontend-localstorage-test-polyfill
f91de51
@dieterolson
Merge branch 'main' into scada/pr8-security-deploy
45e9f27
@dieterolson
Merge branch 'main' into scada/pr6-poll-loop
2b186e9
@dieterolson
Merge branch 'main' into scada/pr11-calibration-alarms
06c66ed
@dieterolson
Merge branch 'main' into scada/pr3-estop-safestate
77cfe95
@dieterolson
Merge branch 'main' into scada/pr4-units-sensors
7bfe956
@dieterolson
Merge branch 'main' into scada/pr7-historian
2f04997
@dieterolson
Merge branch 'main' into scada/pr10-desktop-hmi
8270191
@dieterolson
Merge branch 'main' into scada/pr9-web-hmi
fa74193
@dieterolson
Merge branch 'main' into scada/pr5-control-math
5db2352
@dieterolson
chore(p1am): allowlist generated credential expression
60cb3b3
@dieterolson
test(p1am): keep endpoint suite within file-size budget
cd045a1
@dieterolson
test: remove stale route matrix rows
efb7cba
@dieterolson
Merge branch 'main' into scada/pr8-security-deploy
b6f2e9f
@dieterolson
chore(merge): merge main into scada/pr9-web-hmi
58c1819
@dieterolson
style: apply automated pre-commit formatting and lint fixes
be73777
codex-scheduled and others added 18 commits August 11, 2026 06:27
codex-scheduled
style/fix: pre-commit automated fixes
bb5dfb4
codex-scheduled
style/fix: pre-commit automated fixes
bb30db0
codex-scheduled
style/fix: pre-commit automated fixes
71069aa
codex-scheduled
style/fix: pre-commit automated fixes
a2769bb
codex-scheduled
Merge remote-tracking branch 'origin/scada/pr3-estop-safestate' into …
b1d9453 
…consolidated/p1am-scada-safety-2026-08-13

# Conflicts:
#	SPEC.md
codex-scheduled
Merge remote-tracking branch 'origin/scada/pr4-units-sensors' into co…
9802c47 
…nsolidated/p1am-scada-safety-2026-08-13

# Conflicts:
#	SPEC.md
#	src/p1am_control_system/backend/modbus_client.py
codex-scheduled
Merge remote-tracking branch 'origin/scada/pr5-control-math' into con…
35a8e5b 
…solidated/p1am-scada-safety-2026-08-13

# Conflicts:
#	SPEC.md
#	src/p1am_control_system/backend/modbus_client.py
codex-scheduled
Merge remote-tracking branch 'origin/scada/pr6-poll-loop' into consol…
763ffd6 
…idated/p1am-scada-safety-2026-08-13

# Conflicts:
#	SPEC.md
#	src/p1am_control_system/backend/main.py
#	src/p1am_control_system/backend/modbus_client.py
codex-scheduled
Merge remote-tracking branch 'origin/scada/pr7-historian' into consol…
4578ecb 
…idated/p1am-scada-safety-2026-08-13

# Conflicts:
#	SPEC.md
#	src/p1am_control_system/backend/modbus_client.py
#	src/p1am_control_system/backend/models.py
#	src/p1am_control_system/backend/tests/test_database.py
codex-scheduled
Merge remote-tracking branch 'origin/scada/pr8-security-deploy' into …
83e70ea 
…consolidated/p1am-scada-safety-2026-08-13

# Conflicts:
#	SPEC.md
#	src/p1am_control_system/backend/modbus_client.py
#	src/p1am_control_system/backend/power_supply_integration.py
codex-scheduled
Merge remote-tracking branch 'origin/scada/pr9-web-hmi' into consolid…
d179e71 
…ated/p1am-scada-safety-2026-08-13

# Conflicts:
#	SPEC.md
#	src/p1am_control_system/backend/modbus_client.py
#	src/p1am_control_system/frontend/src/hooks/useTelemetryStream.test.ts
#	src/p1am_control_system/frontend/src/hooks/useTelemetryStream.ts
codex-scheduled
Merge remote-tracking branch 'origin/scada/pr10-desktop-hmi' into con…
7f24165 
…solidated/p1am-scada-safety-2026-08-13

# Conflicts:
#	SPEC.md
#	src/p1am_control_system/backend/modbus_client.py
codex-scheduled
Merge remote-tracking branch 'origin/scada/pr11-calibration-alarms' i…
3a3f084 
…nto consolidated/p1am-scada-safety-2026-08-13

# Conflicts:
#	SPEC.md
#	src/p1am_control_system/backend/main.py
#	src/p1am_control_system/backend/modbus_client.py
#	src/p1am_control_system/backend/settings.py
#	src/p1am_control_system/backend/tests/test_settings.py
codex-scheduled
Merge remote-tracking branch 'origin/fix/frontend-localstorage-test-p…
3940895 
…olyfill' into consolidated/p1am-scada-safety-2026-08-13

# Conflicts:
#	SPEC.md
#	src/p1am_control_system/backend/modbus_client.py
#	src/p1am_control_system/frontend/src/test/setup.ts
codex-scheduled
Merge remote-tracking branch 'origin/fix/p1am-db-path-anchoring' into…
23d3763 
… consolidated/p1am-scada-safety-2026-08-13

# Conflicts:
#	.gitignore
#	SPEC.md
#	src/p1am_control_system/backend/modbus_client.py
#	src/p1am_control_system/backend/tests/test_database.py
codex-scheduled @claude
chore(consolidation): drop unrelated churn and the .codex-worktrees g…
559e8b1 
…itlinks

Every one of the 13 P1AM slice branches ends with two or three
"pre-commit automated fixes" / "main sync" commits that reformatted ~290
files across the whole monorepo. Those commits were produced by an older
pinned ruff than the one CI and .pre-commit-config.yaml now use
(ruff==0.14.10), so merging them re-introduces a formatting regression in
files no P1AM change touches -- and `quality-gate` checks ruff-format on
*changed* files, so each one becomes a gate failure for a change nobody
made.

- 293 Python files outside the P1AM real-content set are restored to
  origin/main exactly. 256 of them were provably pure formatting (ruff
  0.14.10 renders them byte-identical to main); the remainder sit in
  ruff-excluded trees (movement_optimizer, pendulum_simulator,
  data_processing) that main has never had formatted, which is precisely
  why they must go back rather than be reformatted here.
- The six `.codex-worktrees/*` entries are removed. They are git mode
  160000 gitlinks for Codex agent scratch worktrees that got committed by
  accident: there is no .gitmodules declaring them, origin/main has none
  of them, and the commits they name exist only in one local clone and on
  no remote. Merged, a fresh clone of Tools would carry six gitlinks
  pointing at objects the server does not have.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
codex-scheduled @claude
fix(p1am): close four defects the merged slices introduced or left open
85c8086 
Found while integrating the batch; each is in the fail-open direction, and
three of them defeat the very guarantee their own slice was written to
provide. Regression tests accompany all four.

1. state.py — a fabricated reading could clear a live alarm (#4034 vs #4004).
   `_restore_engine_alarms` called `update_tag(tag, float(entry.get("value",
   0.0)))`. `get_active_alarms()` is only contracted to agree between the Rust
   `tools_core.scada` engine and the pure-Python fallback on the keys the
   restore path reads, not on carrying `value` at all; where it is absent the
   substituted 0.0 resolved the tag to Normal. `apply_config` clears
   `active_alarms` first and runs on every routing deploy AND every
   reconnect-time `_publish_active_config`, so a live HiHi was erased from both
   the engine and the live map by a number nothing measured. Composed with the
   poll loop's new "only a real measurement may move the alarm state machine"
   rule the result was worse: the poll loop then refuses to re-evaluate alarms
   while the link is down, so the cleared HiHi stayed cleared for the whole
   outage. A record with no finite value now keeps the state it was snapshotted
   in and never touches the engine.

2. desktop/alarm_state.py — NaN was classified as "normal" (#4012).
   `classify_value` decides the band with four comparisons and every comparison
   against NaN is False, so a NaN fell through to `return None` = inside the
   normal band, and `evaluate` emitted `cleared` transitions and dropped the tag
   from both sets. A garbled or unscaled register therefore silenced a live
   High-High on the heater, and `json.loads` accepts a bare `NaN`, so it was
   reachable straight off the wire. `_require_number` now rejects non-finite
   input and `_evaluate_alarms` catches it alongside TypeError, logging and
   skipping the tag. Skipping is the fail-safe outcome because `evaluate`
   validates before it clears anything, so the latch survives.

3. desktop/header.py — the new annunciator flashed at 10 Hz into a steady light
   (#4012). `set_alarms_state` unconditionally set `_flash_state = True`, and
   `_refresh_annunciator` calls it on EVERY telemetry frame, so the flash
   timer's OFF phase was overwritten within ~100 ms. An unacknowledged alarm
   rendered effectively steady, erasing the flash-vs-steady distinction that
   tells an operator whether anyone has seen it. The phase now restarts only on
   a transition INTO unacknowledged.

4. test_route_authz_matrix.py — the authz exhaustiveness net had a hole punched
   in it (#4028). The slice's last commit deleted 24 rows as "stale", but
   main.py mounts the power-supply and temperature routers unconditionally, so
   14 of those routes do exist: `test_every_route_is_classified` fails outright
   on the merged tree (verified before and after). Those rows are restored —
   they are the DC supply's and the heater's own command surface. The 9 Data
   Explorer rows genuinely are conditional, since that router mounts only when
   its numeric stack imports, so they now register from the live app instead of
   being deleted; that satisfies both the exhaustiveness check and the
   no-stale-rows check in either environment. The suite goes from 51 tests with
   1 failure to 84 passing.

Also repairs one broken test rather than deleting it:
test_estop_shutdown_safe_state.py::test_connect_loop_wakes_immediately_on_shutdown
reused the import-time module-level `shutdown_event`, which asyncio binds to a
loop on first await; earlier tests in the same module had already awaited it on
their own per-test loop, so it raised "bound to a different event loop" before
reaching the assertion. It now patches in a fresh Event.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
codex-scheduled @claude
fix(p1am): rebuild SPEC.md correctly and restore LF line endings
441b196 
Two integration defects in the consolidation itself, both of which buried the
real change under a whole-file rewrite.

SPEC.md was duplicated eleven times. Every slice prepends its own dated section
immediately after "## 3. Goals & Non-Goals" AND deletes the same blank line
further down, so git found no usable anchor and each merge presented one side of
the hunk as the entire remainder of the document. Resolving those by keeping
both sides concatenated the whole spec once per slice: 26548 lines against
main's 2401, with eleven copies of "## 2. Purpose", "## 3. Goals & Non-Goals"
and "## 12. Change Log". SPEC.md is rebuilt as main's file with each slice's §3
subsection inserted exactly once, in the order the work was layered, plus one
§12 change-log row for the consolidation. The diff against main is now purely
additive: +403/-0, eleven `### 2026-07-31 P1AM ...` headings and one table row.
(The three duplicate `###` headings that remain are pre-existing in main.)

Line endings: nine files were carrying CRLF where main uses LF. Only `*.py` is
covered by .gitattributes (`text eol=lf`), so Python normalises itself on
commit; .toml, .ts, .tsx, .sh, Dockerfile and .md store whatever bytes were
committed, and several slices rewrote those wholesale. This was not cosmetic:

- `deploy/launch-hmi.sh` had CRLF, which on the Raspberry Pi fails at exec with
  `bad interpreter: /bin/bash^M` — the operator's HMI launcher would simply not
  start. `deploy/install-services.sh` was already LF and is unaffected.
- `backend/Dockerfile` had CRLF, where a line-continued RUN can break the build.
- The rewrites hid the actual edits behind a full-file diff and would conflict
  against every future change to the same files. pyproject.toml goes from
  +433/-410 to +23/-0 (the `p1am` extra), requirements-lock.txt to +4/-1,
  launch-hmi.sh to +16/-0, App.tsx to +72/-319, setup.ts to +63/-0.

`credentials.ts` and `TuningPanel.tsx` are new on this branch and were also
CRLF; they are normalised to LF to match every existing sibling and the repo's
`core.eol=lf`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Aug 14, 2026
View reviewed changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 441b196797

ℹ️ 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".

Comment thread src/p1am_control_system/backend/database.py
override = os.environ.get("P1AM_DB_PATH", "").strip()
if override:
return str(Path(override).expanduser().resolve())
return str(Path(__file__).resolve().parent / DB_FILENAME)

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Point the container historian at the mounted volume

In the Docker deployment, this fallback resolves to /app/dcs_scada.db because the backend is copied to /app, but docker-compose.yml now mounts dcs_db_data at /data and does not set P1AM_DB_PATH. Consequently, the historian is written to the container layer instead of the named volume, and existing history under /data/dcs_scada.db appears missing after this upgrade while new history is lost on container recreation. Set the container override to a path under /data or make the fallback deployment-aware.

AGENTS.md reference: AGENTS.md:L147-L151

Useful? React with 👍 / 👎.

Comment thread src/p1am_control_system/backend/audit.py
try:
with session_factory() as session:
session.add(event)
session.commit()

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Move audit persistence off the control event loop

Every audited POST/PUT/PATCH/DELETE reaches this synchronous SQLite commit from AuditMiddleware.__call__ on the main ASGI event loop. Under SQLite contention or slow SD-card I/O, the configured 5-second busy timeout can therefore stall PLC scans, heartbeat writes, WebSocket updates, and subsequent E-stop requests; rejected requests are audited too, so an unauthenticated client can repeatedly trigger this path. Queue or dispatch audit writes to a worker instead of committing inline.

AGENTS.md reference: AGENTS.md:L230-L238

Useful? React with 👍 / 👎.

Comment thread src/p1am_control_system/deploy/launch-hmi.sh
if [ -r "$ENV_FILE" ]; then
api_key="$(sed -n 's/^P1AM_API_KEY=//p' "$ENV_FILE" | head -n1)"
if [ -n "${api_key:-}" ]; then
HMI_URL="${HMI_URL}/#apikey=${api_key}"

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep the API key out of the browser command line

When the kiosk launcher finds a credential, embedding it in HMI_URL causes the later Chromium/Firefox exec to retain the operator API key in the browser's process arguments for its entire lifetime. Stripping the fragment from the address bar does not remove it from /proc/<pid>/cmdline, process listings, diagnostics, or crash collection, allowing another local account or process to recover and replay the credential. Seed the browser through a channel that does not expose the secret in argv.

AGENTS.md reference: AGENTS.md:L15-L20

Useful? React with 👍 / 👎.

Comment thread src/p1am_control_system/backend/alicat_manager.py
# PLC drivers that are themselves simulated. Only against one of these may the
# gas subsystem be simulated too; pairing a real PLC with mock MFCs lets an
# operator "establish" a purge that does not exist (issue #4031).
SIMULATED_PLC_DRIVERS = frozenset({"simulator", "simulated"})

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Classify the neural PLC as simulated for MFC setup

With P1AM_PLC_DRIVER=neural, PLCFactory constructs NeuralSimulatorClient and settings.is_simulated_driver() returns true, but this second classifier omits neural. The default mock Alicat configuration is consequently rejected by create_default_manager, leaving /api/alicats empty whenever the supported neural simulator is selected. Reuse the settings classifier or include every simulator driver in one shared predicate.

AGENTS.md reference: AGENTS.md:L122-L127

Useful? React with 👍 / 👎.

@dieterolson
dieterolson enabled auto-merge (squash) August 14, 2026 04:31
codex-scheduled @claude
refactor(p1am): extract the tuning/MPC router so main.py stays in budget
c3a87a2 
`quality-gate` failed on Module Size Budget: `backend/main.py` had reached 1547
lines against its grandfathered baseline of 1440 (hard max 1200). This batch
added the E-stop write-seam refusals, the acknowledged-step requirement and the
shutdown sequencing to that module, and #4064 had already split out
`poll_runtime` for the same reason.

Bumping the baseline would have been the cheap fix and the wrong one — the
baseline exists as a ratchet, and raising it to accommodate growth is the one
thing it is there to prevent. Extracted instead: `tuning_router.py` takes the
one cohesive group left in main.py, the handlers that write to or identify the
dynamics of a single control loop —

  POST /api/tags/{tag_id}
  POST /api/pid/{pid_index}/tuning/start
  POST /api/pid/{pid_index}/tuning/step
  POST /api/pid/{pid_index}/tuning/stop
  POST /api/mpc/simulate

plus `MPCSimulatePayload`, `TagWritePayload` and the `_latest_tag_or_http_error`
helper. main.py: 1547 -> 1341 lines, inside the baseline.

Follows the router-factory pattern already used in this package by
`power_supply_integration.create_power_supply_router` and
`temperature_integration.create_temperature_router`. Collaborators
(control_context, plc_client, backup_simulator, the E-stop refusal callable, the
admin dependency, the logger) are injected rather than imported from `main`,
which would close an import cycle.

Behaviour is unchanged; the routes, paths and admin gating are identical, which
`test_route_authz_matrix.py` verifies from the live app in both directions.

One trap worth recording: the request model must be imported as a real type, not
injected as a parameter. This module uses `from __future__ import annotations`,
so every annotation is a string at runtime and FastAPI resolves it against the
module globals — an injected `step_payload_model` left the annotation as the
literal `"step_payload_model"`, the body never bound, and all six tuning-step
tests returned 422. `models` imports nothing from `main`, so importing
`PIDTuningStepPayload` directly is safe.

P1AM suite: 1388 passed, 6 skipped. ruff check + format clean. Module size
budget passes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 14, 2026
Closed
Closed
Open
codex-scheduled @claude
Merge origin/main (CONS-A1 #4445) into consolidated/p1am-scada-safety…
88f14ce 
…-2026-08-13

`main` moved under this branch when CONS-A1 landed, and the PR went
`mergeable_state=dirty`. With `strict_required_status_checks_policy` true and
`allow_update_branch` false, auto-merge cannot land a behind-or-conflicting
branch — it would have sat silently forever.

One conflict: SPEC.md. Both sides insert into the §12 change-log table directly
under its header. Resolved by rebuilding deterministically rather than by hand:
new main's SPEC.md, plus this batch's eleven §3 P1AM subsections inserted once
each, plus its single §12 row. CONS-A1's three rows are preserved because they
are already in new main. Verified: the diff against `origin/main` is +403/-0 —
purely additive — with one `## 3. Goals & Non-Goals`, one `## 12. Change Log`,
eleven `### 2026-07-31 P1AM` headings and exactly one added table row.

`.gitignore` auto-merged. Note that its diff against main is deliberately
**+9/-4, not zero**: CONS-A1's `.codex-worktrees/` block is present and intact
(this branch never edited it), while the remaining delta is #4068's own change —
three anchored `src/p1am_control_system/backend/dcs_scada.db*` patterns replaced
by unanchored equivalents plus the comment explaining why. A zero diff here
would mean #4068's fix had been dropped.

The four `pdf_renamer` files, `scripts/test_assertion_allowlist.txt` and the two
workflow files CONS-A1 touched come across cleanly and now match `origin/main`
byte-for-byte. This branch had restored them to the previous main during the
churn strip, so its side was unchanged from the merge base and git took theirs
without a conflict.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 14, 2026
Open
Open
codex-scheduled and others added 2 commits August 14, 2026 12:56
codex-scheduled
Merge remote-tracking branch 'origin/main' into consolidated/p1am-sca…
85e40fe 
…da-safety-2026-08-13
codex-scheduled @claude
fix(p1am): ask the router whether a route is served, and make event o…
f08497f 
…rder deterministic

Two CI-only failures on this branch. Neither was a stale table, and in both cases
the easy fix would have cemented a real defect.

1. `test_table_has_no_stale_rows` reported all 19 power-supply / temperature /
   tuning rows as unserved. They ARE served. Proof from inside the same failing
   CI run, single process (`-n 0`): the 19 matching
   `test_gated_route_rejects_anonymous_caller` cases drove those exact paths
   through `TestClient(app)` -- the same module-level `app` object the stale check
   iterates -- and got 401/403. `_DENIED = (401, 403)`, and a route the app does
   not serve answers 404, which would have failed. So the routes were registered
   and reachable and the enumeration was under-reporting them.

   Deleting the rows (as an earlier slice did, which is why
   `test_every_route_is_classified` was failing on the merged tree) would have
   turned the check green while the authz matrix agreed that acknowledge_trip,
   permissive, setpoint, burnout_mode, tc_type and the PID tuning lifecycle did
   not exist -- the plant's entire control surface, in the batch whose review
   theme was "nothing de-energizes".

   `test_table_has_no_stale_rows` now asks `Route.matches()` -- the function
   Starlette itself calls during dispatch -- instead of string-matching
   `route.path` over `app.routes`. A check phrased as "would a request reach a
   handler" cannot disagree with a request. Verified it still discriminates:
   real routes and parameterised routes match, a bogus path does not, and a wrong
   method on a real path does not. `test_every_route_is_classified` deliberately
   stays enumeration-based -- it must catch a NEW route nobody classified, which
   is the direction that ships something ungated.

   Root cause of the enumeration/dispatch divergence is not established; it
   reproduces only under CI's environment (`--import-mode=importlib`, Linux,
   py3.11) and not locally across the identical 101-file selection and flags.
   Phrasing the assertion in dispatch terms makes the test correct regardless.

2. `test_desktop.py::test_event_logger_basic` was flaky, failing roughly one run
   in three locally with `assert 'alarm_trip' == 'alarm_acknowledgment'`. Not
   caused by this batch -- that test file is byte-identical to `main` -- but
   `fetch_logs` is, and it ordered by `timestamp DESC` alone. `datetime.now()`
   reports microseconds while the clock granularity is ~15.6 ms on Windows, so a
   burst of events shares one timestamp string and SQLite may return them in any
   order. Beyond the flake, that means an operator could see an alarm trip listed
   above the acknowledgement that answered it. Now `ORDER BY timestamp DESC, id
   DESC`, using the existing autoincrement primary key, which is monotonic in
   insertion order.

   `test_fetch_logs_orders_same_timestamp_events_newest_first` pins it by writing
   an explicit identical timestamp so the tie is forced rather than hoped for.
   Verified as a real regression test: with the tiebreaker removed it fails,
   returning `['first', 'second', 'third']`.

Also merged `origin/main` (CONS-B1 #4438, 221 files) -- no conflicts. Confirmed
`git grep -n '^from datetime import UTC' -- '*.py'` is empty, so the py3.10
revert #4438 made is intact.

CI-equivalent selection (CI's own 101 files, `-m "not live_simulation"
--import-mode=importlib -n 0`) run three consecutive times: 1949 passed, 6
skipped, 0 failed each time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@dieterolson dieterolson mentioned this pull request Aug 14, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@dieterolson