Skip to content

Review follow-ups: status freshness in fail-safe window, override reason validation#5

Merged
sethpjohnson merged 1 commit into
mainfrom
claude/followups-review-mediums
Jul 17, 2026
Merged

Review follow-ups: status freshness in fail-safe window, override reason validation#5
sethpjohnson merged 1 commit into
mainfrom
claude/followups-review-mediums

Conversation

@sethpjohnson

@sethpjohnson sethpjohnson commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

Stacked on #4 (→ #3#2#1) — the last PR in the stack; merge in order.

Implements the two remaining non-blocking mediums from the principal reviews:

  • /api/status stays fresh during fail-safe recovery: successfully-read temperatures are now recorded during the 'sensor failsafe recovered but restore unconfirmed' window instead of being silently dropped. No failsafe/restore semantics changed — verified by the reviewer against the approved Tier 1 control flow.
  • Override reason validated server-side: ≤128 chars, control characters rejected with 400, free text otherwise (quotes/punctuation/unicode fine — it's operator prose, deliberately not the tight hint-identifier charset). Closes the defense-in-depth asymmetry flagged in the Tier 2 review. ⚠️ Small behavior change: a reason containing a literal newline now gets 400 (previously accepted).
  • Auth tests parameterized across all four mutating routes — a future mutating route registered outside the auth group now fails tests automatically.

Review & verification

  • Principal review → APPROVED (unqualified — first of the session). Reviewer verified both fixes fail-before/pass-after by swapping the pre-fix code back in, confirmed all 16 parameterized sub-tests genuinely execute per suite, and re-confirmed reason has no dashboard innerHTML sink.
  • go vet + go test -race -count=1 ./... clean across all six packages.
  • Separate acceptance drive skipped: both observable behaviors were empirically exercised end-to-end through the real router/control loop by the reviewer's fail-before/pass-after runs; a full app-drive would duplicate that evidence.

Tasks: dex tl6vr184, dex 2cka7723

🤖 Generated with Claude Code

…idation

Two non-blocking mediums from earlier principal reviews:

- controlLoop now records successfully-read temperatures during the
  'sensor failsafe recovered but restore unconfirmed' window (mirroring
  the sticky write-failsafe branch), so /api/status never goes stale
  while the controller is handing control back to the BMC. No failsafe
  or restore semantics changed.
- /api/override reason field is now validated server-side: max 128
  chars, control characters rejected (400), free text otherwise -
  closing the defense-in-depth asymmetry with hint validation without
  restricting human-readable prose. Note: a reason containing a literal
  newline is now rejected with 400 (previously accepted).
- Auth middleware tests parameterized across all four mutating routes,
  so a future route registered outside the auth group fails tests
  automatically.

Tasks: dex tl6vr184, dex 2cka7723

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EQmqj1Hj8DL3GGAcBDNm8v
Base automatically changed from claude/ahl4plme-housekeeping to main July 17, 2026 17:11
@sethpjohnson
sethpjohnson merged commit 3801ee0 into main Jul 17, 2026
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