[cursor]

Risky behavior now covered

• TaskResultLog#update_log: success vs error status from the debug flag, structured JSON payload append, blank-payload no-op, and ApiLog fallback when persistence fails (nightly API update reporting).
• UmApi::TokenStore#fetch: refresh after expiry buffer, missing/failed access_token responses, and exception wrapping into failure results (auth token acquisition for Buildings/Classroom APIs).
• Okta SAML callback: display_name mapping when auth.info.name is nil (regression from Switch shibboleth authentication to okta #461 shibboleth→Okta switch).

Test files added/updated

• spec/lib/task_result_log_spec.rb (new)
• spec/lib/um_api_spec.rb
• spec/requests/users/omniauth_callbacks_spec.rb

Why these tests materially reduce regression risk

Nightly sync depends on correct token refresh/failure signaling and durable success/error logs. A wrong status or dropped structured report hides production failures. Okta’s attribute shape differs from Shibboleth (display_name vs name); without an end-to-end callback assertion, users can be created with blank display names again.

Validation

bundle exec rspec spec/lib/task_result_log_spec.rb spec/lib/um_api_spec.rb spec/requests/users/omniauth_callbacks_spec.rb → 34 examples, 0 failures (SimpleCov minimum on subset runs is expected noise).

  

---

Note

Low Risk
Changes are mostly specs plus clearer debug logging on a failed ApiUpdateLog save; no production auth or sync behavior changes beyond that log message.

Overview
Adds regression coverage around nightly API update logging, UM API token acquisition, and Okta SAML user creation, plus a small fix when ApiUpdateLog persistence fails.

TaskResultLog#update_log (in lib/auth_token_api.rb) now logs real ActiveRecord validation messages via interpolation instead of a static placeholder string. New specs assert success/error status from the debug flag, structured JSON appended to the stored result, no structured section when the payload is blank, and fallback to the API logger when save fails.

UmApi::TokenStore#fetch specs gain shared stubs and cases for post-expiry refresh, success responses without access_token, explicit API failures, and wrapped exceptions (e.g. SocketError).

Okta SAML callback request specs mock auth.info.display_name (with name nil) and assert users get a non-blank display_name after sign-in, matching the post–#461 attribute shape.

^{Reviewed by Cursor Bugbot for commit b1bf929. Bugbot is set up for automated code reviews on this repo. Configure here.}

[Copilot]

🟡 Changes recommended

The fallback spec preserves an unhelpful literal error message, and blank-payload coverage only tests nil.

2 open findings

• Log actual validation errors instead of a literal expression · New
• Test blank payload with an empty hash, not nil · New

What changed in this PR

Adds regression coverage for nightly sync logging, token acquisition, and Okta SAML display names.

Changes:

• Tests TaskResultLog persistence, structured payloads, and fallback logging.
• Expands token caching, expiry, and failure coverage.
• Verifies Okta display_name mapping.

File	Description
spec/​lib/​task_result_log_spec.rb	Covers update-log behavior and fallback logging.
spec/​lib/​um_api_spec.rb	Covers token refresh and failures.
spec/​requests/​users/​omniauth_callbacks_spec.rb	Covers Okta display-name persistence.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit b1bf929. Configure here.}

[cursor]

Stale fallback log spec assertion

Medium Severity

The save-failure fallback now interpolates record.errors.full_messages into the debug message, but the TaskResultLog example still expects the previous uninterpolated text. That spec will fail, so the ApiLog fallback path is no longer covered.

 

^{Reviewed by Cursor Bugbot for commit b1bf929. Configure here.}