[passcod]

🦸 Review Hero

• Run Review Hero
• Run Review Hero on Claude (before merge) Final review with Claude just before merging a substantive PR. Tick one or the other.

[review-hero]

[Design & Architecture] suggestion

lease_is_yours is computed as !lease.holds_for_another(Some(login), now), but holds_for_another is false not only when the lease is the caller's own — it is also false when the lease has no holder at all (held_by is NULL, which the column permits and AnsibleLease::held_by_another's unwrap_or("an operator") fallback anticipates). For such a lease the field reports true, contradicting the field's own doc comment ("Whether that lease is the caller's own"), and the UI hides "Take over" while showing "An operator is running …" for a lease nobody holds — previously that lease offered a take-over, which is the only way to claim it. All current writers pass Some(login) so this needs legacy or hand-edited rows to bite, but the fix is cheap and removes the double negative at the same time: express the predicate positively (e.g. lease.held_by.as_deref() == Some(login), or a holds_for(login) helper beside holds_for_another) so the field means exactly what its documentation says.

[review-hero]

🦸 Review Hero Summary (round 1) · reviewed 8fc078e · on z-ai/glm-5.3-flash:nitro
2 agents reviewed this PR | 0 critical | 1 suggestion | 0 nitpicks

Local fix prompt (copy to your coding agent)

Fix these issues identified on the pull request. One commit per issue fixed.

-------

`crates/private-server/src/fns/ansible.rs:712`: `lease_is_yours` is computed as `!lease.holds_for_another(Some(login), now)`, but `holds_for_another` is false not only when the lease is the caller's own — it is also false when the lease has no holder at all (`held_by` is NULL, which the column permits and `AnsibleLease::held_by_another`'s `unwrap_or("an operator")` fallback anticipates). For such a lease the field reports `true`, contradicting the field's own doc comment ("Whether that lease is the caller's own"), and the UI hides "Take over" while showing "An operator is running …" for a lease nobody holds — previously that lease offered a take-over, which is the only way to claim it. All current writers pass `Some(login)` so this needs legacy or hand-edited rows to bite, but the fix is cheap and removes the double negative at the same time: express the predicate positively (e.g. `lease.held_by.as_deref() == Some(login)`, or a `holds_for(login)` helper beside `holds_for_another`) so the field means exactly what its documentation says.